From 2c16115e236760f3933eadd3a5d7d20dda39866d Mon Sep 17 00:00:00 2001 From: Dan Prince Date: Tue, 6 Sep 2011 07:31:39 -0400 Subject: Update the v1.0 rescue admin action and the v1.1 rescue extension to generate 'adminPass'. Fixes an issue where rescue commands were broken on XenServer. lp#838518 --- nova/api/openstack/contrib/rescue.py | 13 +++++-- nova/api/openstack/create_instance_helper.py | 4 +-- nova/api/openstack/servers.py | 20 +++++++---- nova/compute/api.py | 9 +++-- nova/compute/manager.py | 12 ++++--- nova/flags.py | 3 ++ nova/tests/api/openstack/contrib/test_rescue.py | 23 +++++++++++-- nova/tests/api/openstack/test_server_actions.py | 3 +- nova/tests/api/openstack/test_servers.py | 45 ++++++++++++++++++++----- 9 files changed, 103 insertions(+), 29 deletions(-) diff --git a/nova/api/openstack/contrib/rescue.py b/nova/api/openstack/contrib/rescue.py index 3de128895..f140664b3 100644 --- a/nova/api/openstack/contrib/rescue.py +++ b/nova/api/openstack/contrib/rescue.py @@ -18,11 +18,14 @@ import webob from webob import exc from nova import compute +from nova import flags from nova import log as logging +from nova import utils from nova.api.openstack import extensions as exts from nova.api.openstack import faults +FLAGS = flags.FLAGS LOG = logging.getLogger("nova.api.contrib.rescue") @@ -30,7 +33,7 @@ def wrap_errors(fn): """"Ensure errors are not passed along.""" def wrapped(*args): try: - fn(*args) + return fn(*args) except Exception, e: return faults.Fault(exc.HTTPInternalServerError()) return wrapped @@ -46,9 +49,13 @@ class Rescue(exts.ExtensionDescriptor): def _rescue(self, input_dict, req, instance_id): """Rescue an instance.""" context = req.environ["nova.context"] - self.compute_api.rescue(context, instance_id) + if input_dict['rescue'] and 'adminPass' in input_dict['rescue']: + password = input_dict["rescue"]["adminPass"] + else: + password = utils.generate_password(FLAGS.password_length) + self.compute_api.rescue(context, instance_id, rescue_password=password) - return webob.Response(status_int=202) + return {'adminPass': password} @wrap_errors def _unrescue(self, input_dict, req, instance_id): diff --git a/nova/api/openstack/create_instance_helper.py b/nova/api/openstack/create_instance_helper.py index 29e071609..9c331f2bc 100644 --- a/nova/api/openstack/create_instance_helper.py +++ b/nova/api/openstack/create_instance_helper.py @@ -316,14 +316,14 @@ class CreateInstanceHelper(object): def _get_server_admin_password_old_style(self, server): """ Determine the admin password for a server on creation """ - return utils.generate_password(16) + return utils.generate_password(FLAGS.password_length) def _get_server_admin_password_new_style(self, server): """ Determine the admin password for a server on creation """ password = server.get('adminPass') if password is None: - return utils.generate_password(16) + return utils.generate_password(FLAGS.password_length) if not isinstance(password, basestring) or password == '': msg = _("Invalid adminPass") raise exc.HTTPBadRequest(explanation=msg) diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index 977958f5d..3506a4bca 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -477,16 +477,22 @@ class Controller(object): return webob.Response(status_int=202) @scheduler_api.redirect_handler - def rescue(self, req, id): + def rescue(self, req, id, body={}): """Permit users to rescue the server.""" context = req.environ["nova.context"] try: - self.compute_api.rescue(context, id) + if 'rescue' in body and body['rescue'] and \ + 'adminPass' in body['rescue']: + password = body["rescue"]["adminPass"] + else: + password = utils.generate_password(FLAGS.password_length) + self.compute_api.rescue(context, id, rescue_password=password) except Exception: readable = traceback.format_exc() LOG.exception(_("compute.api::rescue %s"), readable) raise exc.HTTPUnprocessableEntity() - return webob.Response(status_int=202) + + return {'adminPass': password} @scheduler_api.redirect_handler def unrescue(self, req, id): @@ -618,7 +624,7 @@ class ControllerV10(Controller): LOG.debug(msg) raise exc.HTTPBadRequest(explanation=msg) - password = utils.generate_password(16) + password = utils.generate_password(FLAGS.password_length) try: self.compute_api.rebuild(context, instance_id, image_id, password) @@ -760,8 +766,10 @@ class ControllerV11(Controller): self._validate_metadata(metadata) self._decode_personalities(personalities) - password = info["rebuild"].get("adminPass", - utils.generate_password(16)) + if 'rebuild' in info and 'adminPass' in info['rebuild']: + password = info["rebuild"]["adminPass"] + else: + password = utils.generate_password(FLAGS.password_length) try: self.compute_api.rebuild(context, instance_id, image_href, diff --git a/nova/compute/api.py b/nova/compute/api.py index e045ef3de..ca59b5701 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -1271,13 +1271,18 @@ class API(base.Base): self._cast_compute_message('resume_instance', context, instance_id) @scheduler_api.reroute_compute("rescue") - def rescue(self, context, instance_id): + def rescue(self, context, instance_id, rescue_password=None): """Rescue the given instance.""" self.update(context, instance_id, vm_state=vm_states.ACTIVE, task_state=task_states.RESCUING) - self._cast_compute_message('rescue_instance', context, instance_id) + + rescue_params = { + "rescue_password": rescue_password + } + self._cast_compute_message('rescue_instance', context, instance_id, + params=rescue_params) @scheduler_api.reroute_compute("unrescue") def unrescue(self, context, instance_id): diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 0477db745..d5c2f9184 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -70,8 +70,6 @@ flags.DEFINE_string('compute_driver', 'nova.virt.connection.get_connection', 'Driver to use for controlling virtualization') flags.DEFINE_string('stub_network', False, 'Stub network related code') -flags.DEFINE_integer('password_length', 12, - 'Length of generated admin passwords') flags.DEFINE_string('console_host', socket.gethostname(), 'Console proxy host to use to connect to instances on' 'this host.') @@ -796,12 +794,18 @@ class ComputeManager(manager.SchedulerDependentManager): @exception.wrap_exception(notifier=notifier, publisher_id=publisher_id()) @checks_instance_lock - def rescue_instance(self, context, instance_id): - """Rescue an instance on this host.""" + def rescue_instance(self, context, instance_id, **kwargs): + """ + Rescue an instance on this host. + :param rescue_password: password to set on rescue instance + """ + LOG.audit(_('instance %s: rescuing'), instance_id, context=context) context = context.elevated() instance_ref = self.db.instance_get(context, instance_id) + instance_ref.admin_pass = kwargs.get('rescue_password', + utils.generate_password(FLAGS.password_length)) network_info = self._get_instance_nw_info(context, instance_ref) # NOTE(blamar): None of the virt drivers use the 'callback' param diff --git a/nova/flags.py b/nova/flags.py index aa76defe5..971e78807 100644 --- a/nova/flags.py +++ b/nova/flags.py @@ -421,6 +421,9 @@ DEFINE_string('root_helper', 'sudo', DEFINE_bool('use_ipv6', False, 'use ipv6') +DEFINE_integer('password_length', 12, + 'Length of generated instance admin passwords') + DEFINE_bool('monkey_patch', False, 'Whether to log monkey patching') diff --git a/nova/tests/api/openstack/contrib/test_rescue.py b/nova/tests/api/openstack/contrib/test_rescue.py index f8126d461..403bcfd4c 100644 --- a/nova/tests/api/openstack/contrib/test_rescue.py +++ b/nova/tests/api/openstack/contrib/test_rescue.py @@ -16,11 +16,14 @@ import json import webob from nova import compute +from nova import flags from nova import test from nova.tests.api.openstack import fakes +FLAGS = flags.FLAGS -def rescue(self, context, instance_id): + +def rescue(self, context, instance_id, rescue_password=None): pass @@ -34,7 +37,19 @@ class RescueTest(test.TestCase): self.stubs.Set(compute.api.API, "rescue", rescue) self.stubs.Set(compute.api.API, "unrescue", unrescue) - def test_rescue(self): + def test_rescue_with_preset_password(self): + body = {"rescue": {"adminPass": "AABBCC112233"}} + req = webob.Request.blank('/v1.1/123/servers/test_inst/action') + req.method = "POST" + req.body = json.dumps(body) + req.headers["content-type"] = "application/json" + + resp = req.get_response(fakes.wsgi_app()) + self.assertEqual(resp.status_int, 200) + resp_json = json.loads(resp.body) + self.assertEqual("AABBCC112233", resp_json['adminPass']) + + def test_rescue_generates_password(self): body = dict(rescue=None) req = webob.Request.blank('/v1.1/123/servers/test_inst/action') req.method = "POST" @@ -43,6 +58,8 @@ class RescueTest(test.TestCase): resp = req.get_response(fakes.wsgi_app()) self.assertEqual(resp.status_int, 200) + resp_json = json.loads(resp.body) + self.assertEqual(FLAGS.password_length, len(resp_json['adminPass'])) def test_unrescue(self): body = dict(unrescue=None) @@ -52,4 +69,4 @@ class RescueTest(test.TestCase): req.headers["content-type"] = "application/json" resp = req.get_response(fakes.wsgi_app()) - self.assertEqual(resp.status_int, 200) + self.assertEqual(resp.status_int, 202) diff --git a/nova/tests/api/openstack/test_server_actions.py b/nova/tests/api/openstack/test_server_actions.py index b9ef41465..4a215dd74 100644 --- a/nova/tests/api/openstack/test_server_actions.py +++ b/nova/tests/api/openstack/test_server_actions.py @@ -622,7 +622,8 @@ class ServerActionsTestV11(test.TestCase): self.assertEqual(res.status_int, 202) body = json.loads(res.body) self.assertEqual(body['server']['image']['id'], '2') - self.assertEqual(len(body['server']['adminPass']), 16) + self.assertEqual(len(body['server']['adminPass']), + FLAGS.password_length) def test_server_rebuild_rejected_when_building(self): body = { diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py index 1591ea56c..b5116b496 100644 --- a/nova/tests/api/openstack/test_servers.py +++ b/nova/tests/api/openstack/test_servers.py @@ -28,6 +28,7 @@ import webob from nova import context from nova import db from nova import exception +from nova import flags from nova import test from nova import utils import nova.api.openstack @@ -49,6 +50,7 @@ from nova.tests.api.openstack import common from nova.tests.api.openstack import fakes +FLAGS = flags.FLAGS FAKE_UUID = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa' NS = "{http://docs.openstack.org/compute/api/v1.1}" ATOMNS = "{http://www.w3.org/2005/Atom}" @@ -1508,7 +1510,7 @@ class ServersTest(test.TestCase): self.assertEqual(res.status_int, 202) server = json.loads(res.body)['server'] - self.assertEqual(16, len(server['adminPass'])) + self.assertEqual(FLAGS.password_length, len(server['adminPass'])) self.assertEqual('server_test', server['name']) self.assertEqual(1, server['id']) self.assertEqual(2, server['flavorId']) @@ -1709,7 +1711,7 @@ class ServersTest(test.TestCase): self.assertEqual(res.status_int, 202) server = json.loads(res.body)['server'] - self.assertEqual(16, len(server['adminPass'])) + self.assertEqual(FLAGS.password_length, len(server['adminPass'])) self.assertEqual(1, server['id']) self.assertEqual(0, server['progress']) self.assertEqual('server_test', server['name']) @@ -1769,7 +1771,7 @@ class ServersTest(test.TestCase): self.assertEqual(res.status_int, 202) server = json.loads(res.body)['server'] - self.assertEqual(16, len(server['adminPass'])) + self.assertEqual(FLAGS.password_length, len(server['adminPass'])) self.assertEqual(1, server['id']) self.assertEqual("BUILD", server["status"]) self.assertEqual(0, server['progress']) @@ -2480,9 +2482,8 @@ class ServersTest(test.TestCase): self.assertEqual(res.status, '202 Accepted') self.assertEqual(self.server_delete_called, True) - def test_rescue_accepted(self): + def test_rescue_generates_password(self): self.flags(allow_admin_api=True) - body = {} self.called = False @@ -2497,7 +2498,33 @@ class ServersTest(test.TestCase): res = req.get_response(fakes.wsgi_app()) self.assertEqual(self.called, True) - self.assertEqual(res.status_int, 202) + self.assertEqual(res.status_int, 200) + res_body = json.loads(res.body) + self.assertTrue('adminPass' in res_body) + self.assertEqual(FLAGS.password_length, len(res_body['adminPass'])) + + def test_rescue_with_preset_password(self): + self.flags(allow_admin_api=True) + + self.called = False + + def rescue_mock(*args, **kwargs): + self.called = True + + self.stubs.Set(nova.compute.api.API, 'rescue', rescue_mock) + req = webob.Request.blank('/v1.0/servers/1/rescue') + body = {"rescue": {"adminPass": "AABBCC112233"}} + req.body = json.dumps(body) + req.method = 'POST' + req.content_type = 'application/json' + + res = req.get_response(fakes.wsgi_app()) + + self.assertEqual(self.called, True) + self.assertEqual(res.status_int, 200) + res_body = json.loads(res.body) + self.assertTrue('adminPass' in res_body) + self.assertEqual('AABBCC112233', res_body['adminPass']) def test_rescue_raises_handled(self): self.flags(allow_admin_api=True) @@ -3540,7 +3567,8 @@ class TestServerInstanceCreation(test.TestCase): self.assertEquals(response.status_int, 202) response = json.loads(response.body) self.assertTrue('adminPass' in response['server']) - self.assertEqual(16, len(response['server']['adminPass'])) + self.assertEqual(FLAGS.password_length, + len(response['server']['adminPass'])) def test_create_instance_admin_pass_xml(self): request, response, dummy = \ @@ -3549,7 +3577,8 @@ class TestServerInstanceCreation(test.TestCase): dom = minidom.parseString(response.body) server = dom.childNodes[0] self.assertEquals(server.nodeName, 'server') - self.assertEqual(16, len(server.getAttribute('adminPass'))) + self.assertEqual(FLAGS.password_length, + len(server.getAttribute('adminPass'))) class TestGetKernelRamdiskFromImage(test.TestCase): -- cgit From 6fafde1fa52d1eba0c77f403b9e2a6f77b5379cd Mon Sep 17 00:00:00 2001 From: Dan Prince Date: Mon, 12 Sep 2011 09:20:11 -0400 Subject: Make quoting consistent. --- nova/api/openstack/contrib/rescue.py | 2 +- nova/api/openstack/servers.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/nova/api/openstack/contrib/rescue.py b/nova/api/openstack/contrib/rescue.py index f140664b3..2e5dbab73 100644 --- a/nova/api/openstack/contrib/rescue.py +++ b/nova/api/openstack/contrib/rescue.py @@ -50,7 +50,7 @@ class Rescue(exts.ExtensionDescriptor): """Rescue an instance.""" context = req.environ["nova.context"] if input_dict['rescue'] and 'adminPass' in input_dict['rescue']: - password = input_dict["rescue"]["adminPass"] + password = input_dict['rescue']['adminPass'] else: password = utils.generate_password(FLAGS.password_length) self.compute_api.rescue(context, instance_id, rescue_password=password) diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index 49f267eb9..2c6f9724c 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -483,7 +483,7 @@ class Controller(object): try: if 'rescue' in body and body['rescue'] and \ 'adminPass' in body['rescue']: - password = body["rescue"]["adminPass"] + password = body['rescue']['adminPass'] else: password = utils.generate_password(FLAGS.password_length) self.compute_api.rescue(context, id, rescue_password=password) @@ -767,7 +767,7 @@ class ControllerV11(Controller): self._decode_personalities(personalities) if 'rebuild' in info and 'adminPass' in info['rebuild']: - password = info["rebuild"]["adminPass"] + password = info['rebuild']['adminPass'] else: password = utils.generate_password(FLAGS.password_length) -- cgit From 5d5a05bddb59cff6cf5ac1f7104da6030197a9c3 Mon Sep 17 00:00:00 2001 From: Dan Prince Date: Fri, 16 Sep 2011 14:35:10 -0400 Subject: Update test_volumes to use FLAGS.password_length. --- nova/tests/api/openstack/contrib/test_volumes.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/nova/tests/api/openstack/contrib/test_volumes.py b/nova/tests/api/openstack/contrib/test_volumes.py index 443ec399f..52b65f5e1 100644 --- a/nova/tests/api/openstack/contrib/test_volumes.py +++ b/nova/tests/api/openstack/contrib/test_volumes.py @@ -19,6 +19,7 @@ import webob import nova from nova import context +from nova import flags from nova import test from nova.api.openstack.contrib.volumes import BootFromVolumeController from nova.compute import instance_types @@ -26,6 +27,9 @@ from nova.tests.api.openstack import fakes from nova.tests.api.openstack.test_servers import fake_gen_uuid +FLAGS = flags.FLAGS + + def fake_compute_api_create(cls, context, instance_type, image_href, **kwargs): inst_type = instance_types.get_instance_type_by_flavor_id(2) return [{'id': 1, @@ -70,4 +74,4 @@ class BootFromVolumeTest(test.TestCase): self.assertEqual(2, int(server['flavor']['id'])) self.assertEqual(u'test_server', server['name']) self.assertEqual(3, int(server['image']['id'])) - self.assertEqual(16, len(server['adminPass'])) + self.assertEqual(FLAGS.password_length, len(server['adminPass'])) -- cgit