diff --git a/nova/cmd/manage.py b/nova/cmd/manage.py index ee81d5f13b3c..fd6a499b780d 100644 --- a/nova/cmd/manage.py +++ b/nova/cmd/manage.py @@ -2929,7 +2929,7 @@ class VolumeAttachmentCommands(object): im = objects.InstanceMapping.get_by_instance_uuid( ctxt, instance_uuid) with context.target_cell(ctxt, im.cell_mapping) as cctxt: - bdm = objects.BlockDeviceMapping.get_by_volume( + bdm = objects.BlockDeviceMapping.get_by_volume_and_instance( cctxt, volume_id, instance_uuid) if connection_info and json: print(bdm.connection_info) diff --git a/nova/tests/functional/test_nova_manage.py b/nova/tests/functional/test_nova_manage.py index 79f88c38de47..b922792a4143 100644 --- a/nova/tests/functional/test_nova_manage.py +++ b/nova/tests/functional/test_nova_manage.py @@ -261,11 +261,16 @@ class TestNovaManageVolumeAttachmentRefresh( ): """Functional tests for 'nova-manage volume_attachment refresh'.""" + # Required for any multiattach volume tests + microversion = '2.60' + def setUp(self): super().setUp() self.tmpdir = self.useFixture(fixtures.TempDir()).path self.ctxt = context.get_admin_context() self.cli = manage.VolumeAttachmentCommands() + self.output = StringIO() + self.useFixture(fixtures.MonkeyPatch('sys.stdout', self.output)) self.flags(my_ip='192.168.1.100') self.fake_connector = { 'ip': '192.168.1.128', @@ -286,7 +291,7 @@ class TestNovaManageVolumeAttachmentRefresh( self.assertEqual('create', actions[5]['action']) def test_refresh(self): - server = self._create_server() + server = self._create_server(networks='none') volume_id = self.cinder.IMAGE_BACKED_VOL self.api.post_server_volume( server['id'], {'volumeAttachment': {'volumeId': volume_id}}) @@ -344,7 +349,7 @@ class TestNovaManageVolumeAttachmentRefresh( self._assert_instance_actions(server) def test_refresh_rpcapi_remove_volume_connection_rollback(self): - server = self._create_server() + server = self._create_server(networks='none') volume_id = self.cinder.IMAGE_BACKED_VOL self.api.post_server_volume( server['id'], {'volumeAttachment': {'volumeId': volume_id}}) @@ -390,7 +395,7 @@ class TestNovaManageVolumeAttachmentRefresh( self._assert_instance_actions(server) def test_refresh_cinder_attachment_update_rollback(self): - server = self._create_server() + server = self._create_server(networks='none') volume_id = self.cinder.IMAGE_BACKED_VOL self.api.post_server_volume( server['id'], {'volumeAttachment': {'volumeId': volume_id}}) @@ -447,7 +452,7 @@ class TestNovaManageVolumeAttachmentRefresh( def test_refresh_pre_cinderv3_without_attachment_id(self): """Test the refresh command when the bdm has no attachment_id. """ - server = self._create_server() + server = self._create_server(networks='none') volume_id = self.cinder.IMAGE_BACKED_VOL self.api.post_server_volume( server['id'], {'volumeAttachment': {'volumeId': volume_id}}) @@ -489,6 +494,29 @@ class TestNovaManageVolumeAttachmentRefresh( # Assert that we have actions we expect against the instance self._assert_instance_actions(server) + def test_show_multiattach_volume(self): + """Test that the show command doesn't fail for multiattach volumes + """ + volume_id = self.cinder.MULTIATTACH_VOL + + # Launch two instances and attach the same multiattach volume to both + server_1 = self._create_server(networks='none') + self.api.post_server_volume( + server_1['id'], {'volumeAttachment': {'volumeId': volume_id}}) + self._wait_for_volume_attach(server_1['id'], volume_id) + + server_2 = self._create_server(networks='none') + self.api.post_server_volume( + server_2['id'], {'volumeAttachment': {'volumeId': volume_id}}) + self._wait_for_volume_attach(server_2['id'], volume_id) + + result = self.cli.show( + volume_id=volume_id, instance_uuid=server_1['id']) + + # Assert that the command completes successfully, this was previously + # broken and documented under bug #1945452 + self.assertEqual(0, result) + class TestNovaManagePlacementHealAllocations( integrated_helpers.ProviderUsageBaseTestCase): diff --git a/nova/tests/unit/cmd/test_manage.py b/nova/tests/unit/cmd/test_manage.py index cecefa30af0d..927112186280 100644 --- a/nova/tests/unit/cmd/test_manage.py +++ b/nova/tests/unit/cmd/test_manage.py @@ -3111,7 +3111,7 @@ class VolumeAttachmentCommandsTestCase(test.NoDBTestCase): } @mock.patch.object(manage, 'format_dict') - @mock.patch('nova.objects.BlockDeviceMapping.get_by_volume') + @mock.patch('nova.objects.BlockDeviceMapping.get_by_volume_and_instance') @mock.patch('nova.objects.InstanceMapping.get_by_instance_uuid') def test_show(self, mock_get_im, mock_get_bdm, mock_format_dict): """Test the 'show' command.""" @@ -3146,7 +3146,7 @@ class VolumeAttachmentCommandsTestCase(test.NoDBTestCase): mock_format_dict.assert_called_once_with(bdm) @mock.patch('oslo_serialization.jsonutils.dumps') - @mock.patch('nova.objects.BlockDeviceMapping.get_by_volume') + @mock.patch('nova.objects.BlockDeviceMapping.get_by_volume_and_instance') @mock.patch('nova.objects.InstanceMapping.get_by_instance_uuid') def test_show_json(self, mock_get_im, mock_get_bdm, mock_dump): """Test the 'show' command with --json.""" @@ -3181,7 +3181,7 @@ class VolumeAttachmentCommandsTestCase(test.NoDBTestCase): mock_dump.assert_called_once_with(bdm) @mock.patch.object(manage, 'format_dict') - @mock.patch('nova.objects.BlockDeviceMapping.get_by_volume') + @mock.patch('nova.objects.BlockDeviceMapping.get_by_volume_and_instance') @mock.patch('nova.objects.InstanceMapping.get_by_instance_uuid') def test_show_connection_info( self, mock_get_im, mock_get_bdm, mock_format_dict @@ -3225,7 +3225,7 @@ class VolumeAttachmentCommandsTestCase(test.NoDBTestCase): mock_format_dict.assert_called_once_with( fake_connection_info) - @mock.patch('nova.objects.BlockDeviceMapping.get_by_volume') + @mock.patch('nova.objects.BlockDeviceMapping.get_by_volume_and_instance') @mock.patch('nova.objects.InstanceMapping.get_by_instance_uuid') def test_show_connection_info_json(self, mock_get_im, mock_get_bdm): """Test the 'show' command with --json and --connection_info.""" @@ -3300,7 +3300,7 @@ class VolumeAttachmentCommandsTestCase(test.NoDBTestCase): mock.sentinel.context, uuidsentinel.instance) self.assertEqual(2, ret) - @mock.patch('nova.objects.BlockDeviceMapping.get_by_volume') + @mock.patch('nova.objects.BlockDeviceMapping.get_by_volume_and_instance') @mock.patch('nova.objects.InstanceMapping.get_by_instance_uuid') def test_show_bdm_not_found(self, mock_get_im, mock_get_bdm): """Test the 'show' command with a missing bdm."""