Merge "Setting root passwd no longer fails silently."

6 files changed, 61 insertions, 19 deletions

diff --git a/nova/compute/api.py b/nova/compute/api.py
index f78e13f20..860fd1f3a 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -1509,8 +1509,9 @@ class API(base.Base):
                     instance,
                     task_state=task_states.UPDATING_PASSWORD)
 
-        self.compute_rpcapi.set_admin_password(context, instance=instance,
-                new_pass=password)
+        self.compute_rpcapi.set_admin_password(context,
+                                               instance=instance,
+                                               new_pass=password)
 
     @wrap_check_policy
     def inject_file(self, context, instance, path, file_contents):
diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index fd0f9049a..d80781c73 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -1240,7 +1240,9 @@ class ComputeManager(manager.SchedulerDependentManager):
                                       task_state=None)
                 _msg = _('Failed to set admin password. Instance %s is not'
                          ' running') % instance["uuid"]
-                raise exception.Invalid(_msg)
+                raise exception.InstancePasswordSetFailed(
+                                                       instance=instance_uuid,
+                                                       reason=_msg)
             else:
                 try:
                     self.driver.set_admin_password(instance, new_pass)
@@ -1252,23 +1254,29 @@ class ComputeManager(manager.SchedulerDependentManager):
                 except NotImplementedError:
                     # NOTE(dprince): if the driver doesn't implement
                     # set_admin_password we break to avoid a loop
-                    LOG.warn(_('set_admin_password is not implemented '
-                             'by this driver.'), instance=instance)
+                    _msg = _('set_admin_password is not implemented '
+                             'by this driver.')
+                    LOG.warn(_msg, instance=instance)
                     self._instance_update(context,
                                           instance['uuid'],
                                           task_state=None)
-                    break
+                    raise exception.InstancePasswordSetFailed(
+                                                       instance=instance_uuid,
+                                                       reason=_msg)
                 except Exception, e:
                     # Catch all here because this could be anything.
-                    LOG.exception(e, instance=instance)
+                    LOG.exception(_('set_admin_password failed: %s') % e,
+                                  instance=instance)
                     if i == max_tries - 1:
                         self._set_instance_error_state(context,
                                                        instance['uuid'])
                         # We create a new exception here so that we won't
                         # potentially reveal password information to the
                         # API caller.  The real exception is logged above
-                        _msg = _('Error setting admin password')
-                        raise exception.NovaException(_msg)
+                        _msg = _('error setting admin password')
+                        raise exception.InstancePasswordSetFailed(
+                                                       instance=instance_uuid,
+                                                       reason=_msg)
                     time.sleep(1)
                     continue
 
diff --git a/nova/compute/rpcapi.py b/nova/compute/rpcapi.py
index 7bc9ddaf5..c71b6193e 100644
--- a/nova/compute/rpcapi.py
+++ b/nova/compute/rpcapi.py
@@ -411,7 +411,7 @@ class ComputeAPI(nova.openstack.common.rpc.proxy.RpcProxy):
 
     def set_admin_password(self, ctxt, instance, new_pass):
         instance_p = jsonutils.to_primitive(instance)
-        self.cast(ctxt, self.make_msg('set_admin_password',
+        return self.call(ctxt, self.make_msg('set_admin_password',
                 instance=instance_p, new_pass=new_pass),
                 topic=_compute_topic(self.topic, ctxt, None, instance),
                 version='1.33')
diff --git a/nova/exception.py b/nova/exception.py
index 323f33c29..a5a7c0f1b 100644
--- a/nova/exception.py
+++ b/nova/exception.py
@@ -1085,6 +1085,12 @@ class InstanceTypeCreateFailed(NovaException):
     message = _("Unable to create instance type")
 
 
+class InstancePasswordSetFailed(NovaException):
+    message = _("Failed to set admin password on %(instance)s "
+                "because %(reason)s")
+    safe = True
+
+
 class SolidFireAPIException(NovaException):
     message = _("Bad response from SolidFire API")
 
diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py
index 5cee6f94d..d9b87fd15 100644
--- a/nova/tests/compute/test_compute.py
+++ b/nova/tests/compute/test_compute.py
@@ -638,14 +638,15 @@ class ComputeTestCase(BaseTestCase):
         self.stubs.Set(nova.virt.fake.FakeDriver, 'get_info',
                        fake_driver_get_info)
 
-        self.assertRaises(exception.Invalid,
+        self.assertRaises(exception.InstancePasswordSetFailed,
                           self.compute.set_admin_password,
                           self.context,
                           instance=instance)
         self.compute.terminate_instance(self.context, instance['uuid'])
 
-    def test_set_admin_password_driver_error(self):
-        """Ensure error is raised admin password set"""
+    def _do_test_set_admin_password_driver_error(self, exc, expected_vm_state,
+                                                 expected_task_state):
+        """Ensure expected exception is raised if set_admin_password fails"""
 
         def fake_sleep(_time):
             pass
@@ -653,7 +654,7 @@ class ComputeTestCase(BaseTestCase):
         self.stubs.Set(time, 'sleep', fake_sleep)
 
         def fake_driver_set_pass(self2, _instance, _pwd):
-            raise exception.NotAuthorized(_('Internal error'))
+            raise exc
 
         self.stubs.Set(nova.virt.fake.FakeDriver, 'set_admin_password',
                        fake_driver_set_pass)
@@ -669,17 +670,37 @@ class ComputeTestCase(BaseTestCase):
 
         #error raised from the driver should not reveal internal information
         #so a new error is raised
-        self.assertRaises(exception.NovaException,
+        self.assertRaises(exception.InstancePasswordSetFailed,
                           self.compute.set_admin_password,
                           self.context,
                           instance=jsonutils.to_primitive(inst_ref))
 
         inst_ref = db.instance_get_by_uuid(self.context, instance['uuid'])
-        self.assertEqual(inst_ref['vm_state'], vm_states.ERROR)
-        self.assertEqual(inst_ref['task_state'], task_states.UPDATING_PASSWORD)
+        self.assertEqual(inst_ref['vm_state'], expected_vm_state)
+        self.assertEqual(inst_ref['task_state'], expected_task_state)
 
         self.compute.terminate_instance(self.context, inst_ref['uuid'])
 
+    def test_set_admin_password_driver_not_authorized(self):
+        """
+        Ensure expected exception is raised if set_admin_password not
+        authorized.
+        """
+        exc = exception.NotAuthorized(_('Internal error'))
+        self._do_test_set_admin_password_driver_error(exc,
+                                                vm_states.ERROR,
+                                                task_states.UPDATING_PASSWORD)
+
+    def test_set_admin_password_driver_not_implemented(self):
+        """
+        Ensure expected exception is raised if set_admin_password not
+        implemented by driver.
+        """
+        exc = NotImplementedError()
+        self._do_test_set_admin_password_driver_error(exc,
+                                                      vm_states.ACTIVE,
+                                                      None)
+
     def test_inject_file(self):
         """Ensure we can write a file to an instance"""
         called = {'inject': False}
@@ -2710,11 +2731,17 @@ class ComputeAPITestCase(BaseTestCase):
         self.assertEqual(inst_ref['vm_state'], vm_states.ACTIVE)
         self.assertEqual(inst_ref['task_state'], None)
 
+        def fake_rpc_method(context, topic, msg, do_cast=True):
+            self.assertFalse(do_cast)
+
+        self.stubs.Set(rpc, 'call', fake_rpc_method)
+
         self.compute_api.set_admin_password(self.context, inst_ref)
 
         inst_ref = db.instance_get_by_uuid(self.context, instance_uuid)
         self.assertEqual(inst_ref['vm_state'], vm_states.ACTIVE)
-        self.assertEqual(inst_ref['task_state'], task_states.UPDATING_PASSWORD)
+        self.assertEqual(inst_ref['task_state'],
+                         task_states.UPDATING_PASSWORD)
 
         self.compute.terminate_instance(self.context, instance_uuid)
 
diff --git a/nova/tests/compute/test_rpcapi.py b/nova/tests/compute/test_rpcapi.py
index e099a776c..b8e62c2d6 100644
--- a/nova/tests/compute/test_rpcapi.py
+++ b/nova/tests/compute/test_rpcapi.py
@@ -298,7 +298,7 @@ class ComputeRpcAPITestCase(test.TestCase):
                 version='1.32')
 
     def test_set_admin_password(self):
-        self._test_compute_api('set_admin_password', 'cast',
+        self._test_compute_api('set_admin_password', 'call',
                 instance=self.fake_instance, new_pass='pw', version='1.33')
 
     def test_set_host_enabled(self):