openstack/nova
Code Issues Proposed changes

Fix check instance host for instance action

Browse Source 
When instance has no host, actions such as get_console_output,
start_stop_instance cause HTTP 500 response. Here change to
HTTPConflict when action called before host set.

Fix LP# 1116012

Change-Id: I6153a03f449d9fad8d0d8fb7295bdea4d2b2c2b1
This commit is contained in:
Zhou ShaoYu
2013-02-05 14:42:53 +08:00
parent 81da7771aa
commit 702fdf2fc1
6 changed files with 90 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
nova/api/openstack/compute/contrib/console_output.py
View File

@@ -65,6 +65,8 @@ class ConsoleOutputController(wsgi.Controller):
length)
except exception.NotFound:
raise webob.exc.HTTPNotFound(_('Unable to get console'))
except exception.InstanceNotReady as e:
raise webob.exc.HTTPConflict(explanation=unicode(e))
# XML output is not correctly escaped, so remove invalid characters
remove_re = re.compile('[\x00-\x08\x0B-\x0C\x0E-\x1F]')

6
nova/api/openstack/compute/contrib/server_start_stop.py
View File

@@ -44,7 +44,10 @@ class ServerStartStopActionController(wsgi.Controller):
context = req.environ['nova.context']
instance = self._get_instance(context, id)
LOG.debug(_('start instance'), instance=instance)
try:
self.compute_api.start(context, instance)
except exception.InstanceNotReady as e:
raise webob.exc.HTTPConflict(explanation=unicode(e))
return webob.Response(status_int=202)
@wsgi.action('os-stop')
@@ -53,7 +56,10 @@ class ServerStartStopActionController(wsgi.Controller):
context = req.environ['nova.context']
instance = self._get_instance(context, id)
LOG.debug(_('stop instance'), instance=instance)
try:
self.compute_api.stop(context, instance)
except exception.InstanceNotReady as e:
raise webob.exc.HTTPConflict(explanation=unicode(e))
return webob.Response(status_int=202)

26
nova/compute/api.py
View File

@@ -131,6 +131,15 @@ def check_instance_state(vm_state=None, task_state=(None,)):
return outer
def check_instance_host(function):
@functools.wraps(function)
def wrapped(self, context, instance, *args, **kwargs):
if not instance['host']:
raise exception.InstanceNotReady(instance_id=instance['uuid'])
return function(self, context, instance, *args, **kwargs)
return wrapped
def check_instance_lock(function):
@functools.wraps(function)
def inner(self, context, instance, *args, **kwargs):
@@ -1213,6 +1222,7 @@ class API(base.Base):
@wrap_check_policy
@check_instance_lock
@check_instance_host
@check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.RESCUED,
vm_states.ERROR, vm_states.STOPPED],
task_state=[None])
@@ -1231,6 +1241,7 @@ class API(base.Base):
@wrap_check_policy
@check_instance_lock
@check_instance_host
@check_instance_state(vm_state=[vm_states.STOPPED])
def start(self, context, instance):
"""Start an instance."""
@@ -2135,11 +2146,9 @@ class API(base.Base):
file_contents=file_contents)
@wrap_check_policy
@check_instance_host
def get_vnc_console(self, context, instance, console_type):
"""Get a url to an instance Console."""
if not instance['host']:
raise exception.InstanceNotReady(instance_id=instance['uuid'])
connect_info = self.compute_rpcapi.get_vnc_console(context,
instance=instance, console_type=console_type)
@@ -2149,20 +2158,17 @@ class API(base.Base):
return {'url': connect_info['access_url']}
@check_instance_host
def get_vnc_connect_info(self, context, instance, console_type):
"""Used in a child cell to get console info."""
if not instance['host']:
raise exception.InstanceNotReady(instance_id=instance['uuid'])
connect_info = self.compute_rpcapi.get_vnc_console(context,
instance=instance, console_type=console_type)
return connect_info
@wrap_check_policy
@check_instance_host
def get_spice_console(self, context, instance, console_type):
"""Get a url to an instance Console."""
if not instance['host']:
raise exception.InstanceNotReady(instance_id=instance['uuid'])
connect_info = self.compute_rpcapi.get_spice_console(context,
instance=instance, console_type=console_type)
@@ -2172,15 +2178,15 @@ class API(base.Base):
return {'url': connect_info['access_url']}
@check_instance_host
def get_spice_connect_info(self, context, instance, console_type):
"""Used in a child cell to get console info."""
if not instance['host']:
raise exception.InstanceNotReady(instance_id=instance['uuid'])
connect_info = self.compute_rpcapi.get_spice_console(context,
instance=instance, console_type=console_type)
return connect_info
@wrap_check_policy
@check_instance_host
def get_console_output(self, context, instance, tail_length=None):
"""Get console output for an instance."""
return self.compute_rpcapi.get_console_output(context,

16
nova/tests/api/openstack/compute/contrib/test_console_output.py
View File

@@ -35,6 +35,10 @@ def fake_get_console_output(self, _context, _instance, tail_length):
return '\n'.join(fixture)
def fake_get_console_output_not_ready(self, _context, _instance, tail_length):
raise exception.InstanceNotReady(instance_id=_instance["uuid"])
def fake_get(self, context, instance_uuid):
return {'uuid': instance_uuid}
@@ -133,3 +137,15 @@ class ConsoleOutputExtensionTest(test.TestCase):
res = req.get_response(self.app)
self.assertEqual(res.status_int, 400)
def test_get_console_output_not_ready(self):
self.stubs.Set(compute_api.API, 'get_console_output',
fake_get_console_output_not_ready)
body = {'os-getConsoleOutput': {'length': 3}}
req = webob.Request.blank('/v2/fake/servers/1/action')
req.method = "POST"
req.body = jsonutils.dumps(body)
req.headers["content-type"] = "application/json"
res = req.get_response(self.app)
self.assertEqual(res.status_int, 409)

21
nova/tests/api/openstack/compute/contrib/test_server_start_stop.py
View File

@@ -17,6 +17,7 @@ import webob
from nova.api.openstack.compute.contrib import server_start_stop
from nova.compute import api as compute_api
from nova import exception
from nova import test
from nova.tests.api.openstack import fakes
@@ -25,6 +26,10 @@ def fake_compute_api_get(self, context, instance_id):
return {'id': 1, 'uuid': instance_id}
def fake_start_stop_not_ready(self, context, instance):
raise exception.InstanceNotReady(instance_id=instance["uuid"])
class ServerStartStopTest(test.TestCase):
def setUp(self):
@@ -41,6 +46,14 @@ class ServerStartStopTest(test.TestCase):
body = dict(start="")
self.controller._start_server(req, 'test_inst', body)
def test_start_not_ready(self):
self.stubs.Set(compute_api.API, 'get', fake_compute_api_get)
self.stubs.Set(compute_api.API, 'start', fake_start_stop_not_ready)
req = fakes.HTTPRequest.blank('/v2/fake/servers/test_inst/action')
body = dict(start="")
self.assertRaises(webob.exc.HTTPConflict,
self.controller._start_server, req, 'test_inst', body)
def test_stop(self):
self.stubs.Set(compute_api.API, 'get', fake_compute_api_get)
self.mox.StubOutWithMock(compute_api.API, 'stop')
@@ -51,6 +64,14 @@ class ServerStartStopTest(test.TestCase):
body = dict(stop="")
self.controller._stop_server(req, 'test_inst', body)
def test_stop_not_ready(self):
self.stubs.Set(compute_api.API, 'get', fake_compute_api_get)
self.stubs.Set(compute_api.API, 'stop', fake_start_stop_not_ready)
req = fakes.HTTPRequest.blank('/v2/fake/servers/test_inst/action')
body = dict(start="")
self.assertRaises(webob.exc.HTTPConflict,
self.controller._stop_server, req, 'test_inst', body)
def test_start_with_bogus_id(self):
req = fakes.HTTPRequest.blank('/v2/fake/servers/test_inst/action')
body = dict(start="")

27
nova/tests/compute/test_compute.py
View File

@@ -3783,6 +3783,15 @@ class ComputeAPITestCase(BaseTestCase):
db.instance_destroy(self.context, instance['uuid'])
def test_start_no_host(self):
instance = self._create_fake_instance(params={'host': ''})
self.assertRaises(exception.InstanceNotReady,
self.compute_api.start,
self.context, instance)
db.instance_destroy(self.context, instance['uuid'])
def test_stop(self):
instance = jsonutils.to_primitive(self._create_fake_instance())
instance_uuid = instance['uuid']
@@ -3798,6 +3807,15 @@ class ComputeAPITestCase(BaseTestCase):
db.instance_destroy(self.context, instance['uuid'])
def test_stop_no_host(self):
instance = self._create_fake_instance(params={'host': ''})
self.assertRaises(exception.InstanceNotReady,
self.compute_api.stop,
self.context, instance)
db.instance_destroy(self.context, instance['uuid'])
def test_start_shutdown(self):
def check_state(instance_uuid, power_state_, vm_state_, task_state_):
instance = db.instance_get_by_uuid(self.context, instance_uuid)
@@ -5636,6 +5654,15 @@ class ComputeAPITestCase(BaseTestCase):
fake_instance, tail_length=fake_tail_length)
self.assertEqual(output, fake_console_output)
def test_console_output_no_host(self):
instance = self._create_fake_instance(params={'host': ''})
self.assertRaises(exception.InstanceNotReady,
self.compute_api.get_console_output,
self.context, instance)
db.instance_destroy(self.context, instance['uuid'])
def test_attach_volume(self):
# Ensure instance can be soft rebooted.