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

Also relocate 'password_length' flag and replace cases where password length was hard-coded.

10 files changed, 108 insertions, 30 deletions

diff --git a/nova/api/openstack/contrib/rescue.py b/nova/api/openstack/contrib/rescue.py
index 3de128895..2e5dbab73 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 e27ddf78b..79f17e27f 100644
--- a/nova/api/openstack/create_instance_helper.py
+++ b/nova/api/openstack/create_instance_helper.py
@@ -317,14 +317,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 5affd1f33..0ef246852 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 4220f47ae..95ea13f61 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -1274,13 +1274,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 7915830ec..b5de6f048 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.')
@@ -797,12 +795,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/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']))
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 ee7927c64..a2c8b3b04 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}"
@@ -1575,7 +1577,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'])
@@ -1776,7 +1778,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'])
@@ -1836,7 +1838,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'])
@@ -2547,9 +2549,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
 
@@ -2564,7 +2565,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')
+        req.method = 'POST'
+        body = {"rescue": {"adminPass": "AABBCC112233"}}
+        req.body = json.dumps(body)
+        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)
@@ -3621,7 +3648,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 = \
@@ -3630,7 +3658,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):