diff options
author | Zuul <zuul@review.opendev.org> | 2020-09-17 09:59:46 +0000 |
---|---|---|
committer | Gerrit Code Review <review@openstack.org> | 2020-09-17 09:59:47 +0000 |
commit | d9dbbf0df3e0a4abb74f64803a58df7d4cbb5ee7 (patch) | |
tree | 0290b7e3853ac73f29dd88d61d3df0edcd0daf86 | |
parent | 4ae15546a1a6c4ba47b367b6c030710530f39e03 (diff) | |
parent | 4130ef8af0331bd1ec363b85c3c5cd827e6ea6c7 (diff) | |
download | nova-d9dbbf0df3e0a4abb74f64803a58df7d4cbb5ee7.tar.gz |
Merge "Add checks for volume status when rebuilding" into stable/stein
-rw-r--r-- | nova/api/openstack/compute/servers.py | 1 | ||||
-rw-r--r-- | nova/compute/api.py | 22 | ||||
-rw-r--r-- | nova/tests/unit/api/openstack/compute/test_server_actions.py | 15 | ||||
-rw-r--r-- | nova/tests/unit/compute/test_compute_api.py | 60 |
4 files changed, 94 insertions, 4 deletions
diff --git a/nova/api/openstack/compute/servers.py b/nova/api/openstack/compute/servers.py index 76b131b268..63c407433b 100644 --- a/nova/api/openstack/compute/servers.py +++ b/nova/api/openstack/compute/servers.py @@ -1104,6 +1104,7 @@ class ServersController(wsgi.Controller): exception.ImageNotActive, exception.ImageUnacceptable, exception.InvalidMetadata, + exception.InvalidVolume, ) as error: raise exc.HTTPBadRequest(explanation=error.format_message()) except INVALID_FLAVOR_IMAGE_EXCEPTIONS as error: diff --git a/nova/compute/api.py b/nova/compute/api.py index 8e097b0f0b..d4ad8d17ba 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -3424,6 +3424,12 @@ class API(base.Base): self._checks_for_create_and_rebuild(context, image_id, image, flavor, metadata, files_to_inject, root_bdm) + # Check the state of the volume. If it is not in-use, an exception + # will occur when creating attachment during reconstruction, + # resulting in the failure of reconstruction and the instance + # turning into an error state. + self._check_volume_status(context, bdms) + # NOTE(sean-k-mooney): When we rebuild with a new image we need to # validate that the NUMA topology does not change as we do a NOOP claim # in resource tracker. As such we cannot allow the resource usage or @@ -3524,6 +3530,17 @@ class API(base.Base): request_spec=request_spec, kwargs=kwargs) + def _check_volume_status(self, context, bdms): + """Check whether the status of the volume is "in-use". + + :param context: A context.RequestContext + :param bdms: BlockDeviceMappingList of BDMs for the instance + """ + for bdm in bdms: + if bdm.volume_id: + vol = self.volume_api.get(context, bdm.volume_id) + self.volume_api.check_attached(context, vol) + @staticmethod def _validate_numa_rebuild(instance, image, flavor): """validates that the NUMA constraints do not change on rebuild. @@ -3970,10 +3987,7 @@ class API(base.Base): bdms = objects.BlockDeviceMappingList.get_by_instance_uuid( context, instance.uuid) - for bdm in bdms: - if bdm.volume_id: - vol = self.volume_api.get(context, bdm.volume_id) - self.volume_api.check_attached(context, vol) + self._check_volume_status(context, bdms) if compute_utils.is_volume_backed_instance(context, instance, bdms): reason = _("Cannot rescue a volume-backed instance") raise exception.InstanceNotRescuable(instance_id=instance.uuid, diff --git a/nova/tests/unit/api/openstack/compute/test_server_actions.py b/nova/tests/unit/api/openstack/compute/test_server_actions.py index b9c9685da6..4feafc5186 100644 --- a/nova/tests/unit/api/openstack/compute/test_server_actions.py +++ b/nova/tests/unit/api/openstack/compute/test_server_actions.py @@ -640,6 +640,21 @@ class ServerActionsControllerTestV21(test.TestCase): self.controller._action_rebuild, self.req, FAKE_UUID, body=body) + @mock.patch.object(compute_api.API, 'rebuild') + def test_rebuild_raise_invalid_volume_exc(self, mock_rebuild): + """Make sure that we can't rebuild with an InvalidVolume exception.""" + body = { + "rebuild": { + "imageRef": self._image_href, + }, + } + + mock_rebuild.side_effect = exception.InvalidVolume('error') + + self.assertRaises(webob.exc.HTTPBadRequest, + self.controller._action_rebuild, + self.req, FAKE_UUID, body=body) + def test_resize_server(self): body = dict(resize=dict(flavorRef="http://localhost/3")) diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index 719617de85..017dbb0cd6 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -3771,6 +3771,66 @@ class _ComputeAPIUnitTestMixIn(object): @mock.patch.object(compute_api.API, '_check_auto_disk_config') @mock.patch.object(compute_api.API, '_checks_for_create_and_rebuild') @mock.patch.object(compute_api.API, '_record_action_start') + def test_rebuild_with_invalid_volume(self, _record_action_start, + _checks_for_create_and_rebuild, _check_auto_disk_config, + mock_get_image, mock_get_bdms, get_flavor, + instance_save, req_spec_get_by_inst_uuid): + """Test a negative scenario where the instance has an + invalid volume. + """ + instance = fake_instance.fake_instance_obj( + self.context, vm_state=vm_states.ACTIVE, cell_name='fake-cell', + launched_at=timeutils.utcnow(), + system_metadata={}, image_ref='foo', + expected_attrs=['system_metadata']) + + bdms = objects.BlockDeviceMappingList(objects=[ + objects.BlockDeviceMapping( + boot_index=None, image_id=None, + source_type='volume', destination_type='volume', + volume_type=None, snapshot_id=None, + volume_id=uuids.volume_id, volume_size=None)]) + mock_get_bdms.return_value = bdms + + get_flavor.return_value = test_flavor.fake_flavor + flavor = instance.get_flavor() + + image_href = 'foo' + image = { + "min_ram": 10, "min_disk": 1, + "properties": { + 'architecture': fields_obj.Architecture.X86_64}} + mock_get_image.return_value = (None, image) + + fake_spec = objects.RequestSpec() + req_spec_get_by_inst_uuid.return_value = fake_spec + + fake_volume = {'id': uuids.volume_id, 'status': 'retyping'} + with mock.patch.object(self.compute_api.volume_api, 'get', + return_value=fake_volume) as mock_get_volume: + self.assertRaises(exception.InvalidVolume, + self.compute_api.rebuild, + self.context, + instance, + image_href, + "new password") + self.assertIsNone(instance.task_state) + mock_get_bdms.assert_called_once_with(self.context, + instance.uuid) + mock_get_volume.assert_called_once_with(self.context, + uuids.volume_id) + _check_auto_disk_config.assert_called_once_with(image=image) + _checks_for_create_and_rebuild.assert_called_once_with( + self.context, None, image, flavor, {}, [], None) + + @mock.patch.object(objects.RequestSpec, 'get_by_instance_uuid') + @mock.patch.object(objects.Instance, 'save') + @mock.patch.object(objects.Instance, 'get_flavor') + @mock.patch.object(objects.BlockDeviceMappingList, 'get_by_instance_uuid') + @mock.patch.object(compute_api.API, '_get_image') + @mock.patch.object(compute_api.API, '_check_auto_disk_config') + @mock.patch.object(compute_api.API, '_checks_for_create_and_rebuild') + @mock.patch.object(compute_api.API, '_record_action_start') def test_rebuild(self, _record_action_start, _checks_for_create_and_rebuild, _check_auto_disk_config, _get_image, bdm_get_by_instance_uuid, get_flavor, instance_save, |