From 8dada6d0f66f6c53a78725fb54f6a8e5d36fb127 Mon Sep 17 00:00:00 2001 From: ericxiett Date: Thu, 5 Mar 2020 07:49:38 +0800 Subject: [PATCH] Catch exception when use invalid architecture of image Currently, when attempting to rebuild an instance with an image with invalid architecture, the API raises a 500 error and leaves the instance stuck in the REBUILDING task state.This patch adds checking image's architecture before updating instance's task_state. And catches exception.InvalidArchitectureName then returns HTTPBadRequest. Change-Id: I25eff0271c856a8d3e83867b448e1dec6f6732ab Closes-Bug: #1861749 --- nova/api/openstack/compute/servers.py | 1 + nova/compute/api.py | 7 +++++ .../openstack/compute/test_server_actions.py | 14 ++++++++++ nova/tests/unit/compute/test_compute_api.py | 28 +++++++++++++++++++ 4 files changed, 50 insertions(+) diff --git a/nova/api/openstack/compute/servers.py b/nova/api/openstack/compute/servers.py index 8aad815e33..8dc91ca2e2 100644 --- a/nova/api/openstack/compute/servers.py +++ b/nova/api/openstack/compute/servers.py @@ -1121,6 +1121,7 @@ class ServersController(wsgi.Controller): exception.ImageNotActive, exception.ImageUnacceptable, exception.InvalidMetadata, + exception.InvalidArchitectureName, ) 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 a938669aca..b9dd65fae0 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -3359,6 +3359,12 @@ class API(base.Base): block_device_info=None, reboot_type='HARD') + def _check_image_arch(self, image=None): + if image: + img_arch = image.get("properties", {}).get('hw_architecture') + if img_arch: + fields_obj.Architecture.canonicalize(img_arch) + # TODO(stephenfin): We should expand kwargs out to named args @check_instance_lock @check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.STOPPED, @@ -3405,6 +3411,7 @@ class API(base.Base): image_id, image = self._get_image(context, image_href) self._check_auto_disk_config(image=image, auto_disk_config=auto_disk_config) + self._check_image_arch(image=image) flavor = instance.get_flavor() bdms = objects.BlockDeviceMappingList.get_by_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 d1eaf32ec7..87b77b2221 100644 --- a/nova/tests/unit/api/openstack/compute/test_server_actions.py +++ b/nova/tests/unit/api/openstack/compute/test_server_actions.py @@ -658,6 +658,20 @@ 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_architecture_exc(self, mock_rebuild): + body = { + "rebuild": { + "imageRef": self._image_href, + }, + } + + mock_rebuild.side_effect = exception.InvalidArchitectureName('arm64') + + 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 a557bf7487..67684c96b2 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -3579,6 +3579,19 @@ class _ComputeAPIUnitTestMixIn(object): lambda obj, context, image_id, **kwargs: self.fake_image) return self.fake_image['id'] + def _setup_fake_image_with_invalid_arch(self): + self.fake_image = { + 'id': 2, + 'name': 'fake_name', + 'status': 'active', + 'properties': {"hw_architecture": "arm64"}, + } + + fake_image.stub_out_image_service(self) + self.stub_out('nova.tests.unit.image.fake._FakeImageService.show', + lambda obj, context, image_id, **kwargs: self.fake_image) + return self.fake_image['id'] + @mock.patch('nova.compute.api.API.get_instance_host_status', new=mock.Mock(return_value=fields_obj.HostStatus.UP)) def test_resize_with_disabled_auto_disk_config_fails(self): @@ -3609,6 +3622,21 @@ class _ComputeAPIUnitTestMixIn(object): "new password", auto_disk_config=True) + def test_rebuild_with_invalid_image_arch(self): + 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']) + image_id = self._setup_fake_image_with_invalid_arch() + self.assertRaises(exception.InvalidArchitectureName, + self.compute_api.rebuild, + self.context, + instance, + image_id, + "new password") + self.assertIsNone(instance.task_state) + @mock.patch.object(objects.RequestSpec, 'get_by_instance_uuid') @mock.patch.object(objects.Instance, 'save') @mock.patch.object(objects.Instance, 'get_flavor')