Merge lp:~gundlach/nova/get-server-rename-working into lp:~hudson-openstack/nova/trunk

[Michael Gundlach]

Fix bugs that prevented OpenStack API from supporting server rename.

[Eric Day (eday)]

For line 91, can you have it use the self.compute_api.update_instance method instead? I'm trying to get all DB access out of the API servers. :)

review: Needs Fixing

[Michael Gundlach (gundlach)]

Eric: done :)

[Eric Day (eday)]

Thanks, looks good!

review: Approve

[Sandy Walsh (sandy-walsh)]

Looks good to me. Learned lots in the review too!

review: Approve

[OpenStack Infra (hudson-openstack)]

The attempt to merge lp:~gundlach/nova/get-server-rename-working into lp:nova failed. Below is the output from the failed tests.

nova/tests/api/openstack/test_servers.py:51:26: E225 missing whitespace around operator
    return Instance(id=id+123456, state=0, image_id=10, user_id=user_id,
                         ^
    - Always surround these binary operators with a single space on
      either side: assignment (=), augmented assignment (+=, -= etc.),
      comparisons (==, <, >, !=, <>, <=, >=, in, not in, is, is not),
      Booleans (and, or, not).

    - Use spaces around arithmetic operators.

    Okay: i = i + 1
    Okay: submitted += 1
    Okay: x = x * 2 - 1
    Okay: hypot2 = x * x + y * y
    Okay: c = (a + b) * (a - b)
    Okay: foo(bar, key='word', *args, **kwargs)
    Okay: baz(**kwargs)
    Okay: negative = -1
    Okay: spam(-1)
    Okay: alpha[:-i]
    Okay: if not -5 < x < +5:\n pass
    Okay: lambda *args, **kw: (args, kw)

    E225: i=i+1
    E225: submitted +=1
    E225: x = x*2 - 1
    E225: hypot2 = x*x + y*y
    E225: c = (a+b) * (a-b)
    E225: c = alpha -4
    E225: z = x **y

[OpenStack Infra (hudson-openstack)]

There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions.

=== modified file 'nova/api/openstack/__init__.py'
--- nova/api/openstack/__init__.py	2010-10-25 16:22:29 +0000
+++ nova/api/openstack/__init__.py	2010-12-02 21:36:43 +0000
@@ -25,6 +25,7 @@
 
 import logging
 import routes
+import traceback
 import webob.dec
 import webob.exc
 import webob
@@ -61,6 +62,7 @@
             return req.get_response(self.application)
         except Exception as ex:
             logging.warn("Caught error: %s" % str(ex))
+            logging.debug(traceback.format_exc())
             exc = webob.exc.HTTPInternalServerError(explanation=str(ex))
             return faults.Fault(exc)
 

=== modified file 'nova/api/openstack/servers.py'
--- nova/api/openstack/servers.py	2010-12-02 16:47:25 +0000
+++ nova/api/openstack/servers.py	2010-12-02 21:36:43 +0000
@@ -15,8 +15,6 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-import time
-
 import webob
 from webob import exc
 
@@ -36,16 +34,6 @@
 FLAGS = flags.FLAGS
 
 
-def _filter_params(inst_dict):
-    """ Extracts all updatable parameters for a server update request """
-    keys = dict(name='name', admin_pass='adminPass')
-    new_attrs = {}
-    for k, v in keys.items():
-        if v in inst_dict:
-            new_attrs[k] = inst_dict[v]
-    return new_attrs
-
-
 def _entity_list(entities):
     """ Coerces a list of servers into proper dictionary format """
     return dict(servers=entities)
@@ -64,7 +52,7 @@
     inst_dict = {}
 
     mapped_keys = dict(status='state', imageId='image_id',
-        flavorId='instance_type', name='display_name', id='id')
+        flavorId='instance_type', name='display_name', id='internal_id')
 
     for k, v in mapped_keys.iteritems():
         inst_dict[k] = inst[v]
@@ -79,7 +67,7 @@
 
 def _entity_inst(inst):
     """ Filters all model attributes save for id and name """
-    return dict(server=dict(id=inst['id'], name=inst['display_name']))
+    return dict(server=dict(id=inst['internal_id'], name=inst['display_name']))
 
 
 class Controller(wsgi.Controller):
@@ -89,7 +77,7 @@
         'application/xml': {
             "attributes": {
                 "server": ["id", "imageId", "name", "flavorId", "hostId",
-                           "status", "progress", "progress"]}}}
+                           "status", "progress"]}}}
 
     def __init__(self, db_driver=None):
         if not db_driver:
@@ -173,10 +161,14 @@
         if not instance or instance.user_id != user_id:
             return faults.Fault(exc.HTTPNotFound())
 
-        self.db_driver.instance_update(ctxt,
-                                       int(id),
-                                       _filter_params(inst_dict['server']))
-        return faults.Fault(exc.HTTPNoContent())
+        update_dict = {}
+        if 'adminPass' in inst_dict['server']:
+            update_dict['admin_pass'] = inst_dict['server']['adminPass']
+        if 'name' in inst_dict['server']:
+            update_dict['display_name'] = inst_dict['server']['name']
+
+        self.compute_api.update_instance(ctxt, instance['id'], update_dict)
+        return exc.HTTPNoContent()
 
     def action(self, req, id):
         """ multi-purpose method used to reboot, rebuild, and
@@ -191,6 +183,8 @@
         inst_ref = self.db.instance_get_by_internal_id(ctxt, int(id))
         if not inst_ref or (inst_ref and not inst_ref.user_id == user_id):
             return faults.Fault(exc.HTTPUnprocessableEntity())
+        #TODO(gundlach): pass reboot_type, support soft reboot in
+        #virt driver
         cloud.reboot(id)
 
     def _get_network_topic(self, context):

=== modified file 'nova/compute/manager.py'
--- nova/compute/manager.py	2010-12-02 17:34:52 +0000
+++ nova/compute/manager.py	2010-12-02 21:36:43 +0000
@@ -22,8 +22,8 @@
 The :py:class:`ComputeManager` class is a :py:class:`nova.manager.Manager` that
 handles RPC calls relating to creating instances.  It is responsible for
 building a disk image, launching it via the underlying virtualization driver,
-responding to calls to check it state, attaching persistent as well as
-termination.
+responding to calls to check its state, attaching persistent storage, and
+terminating it.
 
 **Related Flags**
 
@@ -39,7 +39,6 @@
 
 from twisted.internet import defer
 
-from nova import db
 from nova import exception
 from nova import flags
 from nova import manager
@@ -50,10 +49,11 @@
 flags.DEFINE_string('instances_path', '$state_path/instances',
                     'where instances are stored on disk')
 flags.DEFINE_string('compute_driver', 'nova.virt.connection.get_connection',
-                    'Driver to use for volume creation')
+                    'Driver to use for controlling virtualization')
 
 
 class ComputeManager(manager.Manager):
+
     """Manages the running instances from creation to destruction."""
 
     def __init__(self, compute_driver=None, *args, **kwargs):
@@ -93,7 +93,6 @@
         if instance_ref['name'] in self.driver.list_instances():
             raise exception.Error("Instance has already been created")
         logging.debug("instance %s: starting...", instance_id)
-        project_id = instance_ref['project_id']
         self.network_manager.setup_compute_network(context, instance_id)
         self.db.instance_update(context,
                                 instance_id,
@@ -135,7 +134,6 @@
             self.db.instance_destroy(context, instance_id)
             raise exception.Error('trying to destroy already destroyed'
                                   ' instance: %s' % instance_id)
-
         yield self.driver.destroy(instance_ref)
 
         # TODO(ja): should we keep it in a terminated state for a bit?

=== modified file 'nova/tests/api/openstack/test_servers.py'
--- nova/tests/api/openstack/test_servers.py	2010-12-02 16:47:25 +0000
+++ nova/tests/api/openstack/test_servers.py	2010-12-02 21:36:43 +0000
@@ -48,8 +48,8 @@
 
 
 def stub_instance(id, user_id=1):
-    return Instance(id=id, state=0, image_id=10, display_name='server%s' % id,
-                    user_id=user_id)
+    return Instance(id=id + 123456, state=0, image_id=10, user_id=user_id,
+                    display_name='server%s' % id, internal_id=id)
 
 
 class ServersTest(unittest.TestCase):

=== modified file 'nova/virt/xenapi.py'
--- nova/virt/xenapi.py	2010-11-22 13:11:00 +0000
+++ nova/virt/xenapi.py	2010-12-02 21:36:43 +0000
@@ -52,11 +52,9 @@
 
 from twisted.internet import defer
 from twisted.internet import reactor
-from twisted.internet import task
 
 from nova import db
 from nova import flags
-from nova import process
 from nova import utils
 from nova.auth.manager import AuthManager
 from nova.compute import instance_types