Moves db writes into compute manager class. Cleans up sqlalchemy model/api to remove redundant calls for updating what is really a dict.

7 files changed, 104 insertions, 80 deletions

diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py
index 6d4f58499..096ddf668 100644
--- a/nova/api/ec2/cloud.py
+++ b/nova/api/ec2/cloud.py
@@ -97,6 +97,7 @@ class CloudController(object):
 """
     def __init__(self):
         self.network_manager = utils.import_object(FLAGS.network_manager)
+        self.compute_manager = utils.import_object(FLAGS.compute_manager)
         self.setup()
 
     def __str__(self):
@@ -846,27 +847,29 @@ class CloudController(object):
         elevated = context.elevated()
 
         for num in range(num_instances):
-            instance_ref = db.instance_create(context, base_options)
-            inst_id = instance_ref['id']
+            
+            instance_data = base_options
+            instance_data['mac_address'] = utils.generate_mac()
+            instance_data['launch_index'] = num
 
-            for security_group_id in security_groups:
-                db.instance_add_security_group(elevated,
-                                               inst_id,
-                                               security_group_id)
+            instance_ref = self.compute_manager.create_instance(context,
+                                                           instance_data,
+                                                           security_groups)
 
-            inst = {}
-            inst['mac_address'] = utils.generate_mac()
-            inst['launch_index'] = num
             internal_id = instance_ref['internal_id']
             ec2_id = internal_id_to_ec2_id(internal_id)
-            inst['hostname'] = ec2_id
-            db.instance_update(context, inst_id, inst)
+            instance_ref['hostname'] = ec2_id
+
+            self.compute_manager.update_instance(context,
+                                                 instance_ref['id'],
+                                                 instance_ref)
+
             # TODO(vish): This probably should be done in the scheduler
             #             or in compute as a call.  The network should be
             #             allocated after the host is assigned and setup
             #             can happen at the same time.
             address = self.network_manager.allocate_fixed_ip(context,
-                                                             inst_id,
+                                                             instance_ref['id'],
                                                              vpn)
             network_topic = self._get_network_topic(context)
             rpc.cast(elevated,
@@ -878,9 +881,9 @@ class CloudController(object):
                      FLAGS.scheduler_topic,
                      {"method": "run_instance",
                       "args": {"topic": FLAGS.compute_topic,
-                               "instance_id": inst_id}})
+                               "instance_id": instance_ref['id']}})
             logging.debug("Casting to scheduler for %s/%s's instance %s" %
-                      (context.project.name, context.user.name, inst_id))
+                      (context.project.name, context.user.name, instance_ref['id']))
         return self._format_run_instances(context, reservation_id)
 
 
@@ -907,11 +910,13 @@ class CloudController(object):
                               id_str)
                 continue
             now = datetime.datetime.utcnow()
-            db.instance_update(context,
-                               instance_ref['id'],
-                               {'state_description': 'terminating',
-                                'state': 0,
-                                'terminated_at': now})
+            updated_data = {'state_description': 'terminating',
+                            'state': 0,
+                            'terminated_at': now}
+            self.compute_manager.update_instance(context,
+                                                 instance_ref['id'],
+                                                 updated_data)
+
             # FIXME(ja): where should network deallocate occur?
             address = db.instance_get_floating_address(context,
                                                        instance_ref['id'])
diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py
index a73591ccc..6ce364eb7 100644
--- a/nova/api/openstack/servers.py
+++ b/nova/api/openstack/servers.py
@@ -94,6 +94,7 @@ class Controller(wsgi.Controller):
             db_driver = FLAGS.db_driver
         self.db_driver = utils.import_object(db_driver)
         self.network_manager = utils.import_object(FLAGS.network_manager)
+        self.compute_manager = utils.import_object(FLAGS.compute_manager)
         super(Controller, self).__init__()
 
     def index(self, req):
@@ -241,34 +242,30 @@ class Controller(wsgi.Controller):
         inst['memory_mb'] = flavor['memory_mb']
         inst['vcpus'] = flavor['vcpus']
         inst['local_gb'] = flavor['local_gb']
-
-        ref = self.db_driver.instance_create(ctxt, inst)
-        inst['id'] = ref.internal_id
-
         inst['mac_address'] = utils.generate_mac()
-
-        #TODO(dietz) is this necessary?
         inst['launch_index'] = 0
 
-        inst['hostname'] = str(ref.internal_id)
-        self.db_driver.instance_update(ctxt, inst['id'], inst)
+        ref = self.compute_manager.create_instance(ctxt, inst)
+        inst['id'] = ref['internal_id']
+
+        inst['hostname'] = str(ref['internal_id'])
+        self.compute_manager.update_instance(ctxt, inst['id'], inst)
 
-        network_manager = utils.import_object(FLAGS.network_manager)
-        address = network_manager.allocate_fixed_ip(ctxt,
-            inst['id'])
+        address = self.network_manager.allocate_fixed_ip(ctxt,
+                                                         inst['id'])
 
         # TODO(vish): This probably should be done in the scheduler
         #             network is setup when host is assigned
-        network_topic = self._get_network_topic(ctxt, network_manager)
+        network_topic = self._get_network_topic(ctxt)
         rpc.call(ctxt,
                  network_topic,
                  {"method": "setup_fixed_ip",
                   "args": {"address": address}})
         return inst
 
-    def _get_network_topic(self, context, network_manager):
+    def _get_network_topic(self, context):
         """Retrieves the network host for a project"""
-        network_ref = network_manager.get_network(context)
+        network_ref = self.network_manager.get_network(context)
         host = network_ref['host']
         if not host:
             host = rpc.call(context,
diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index 523bb8893..c752d954b 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -67,6 +67,41 @@ class ComputeManager(manager.Manager):
     def refresh_security_group(self, context, security_group_id, **_kwargs):
         yield self.driver.refresh_security_group(security_group_id)
 
+
+    def create_instance(self, context, instance_data, security_groups=[]):
+        """Creates the instance in the datastore and returns the
+        new instance as a mapping
+
+        :param context: The security context
+        :param instance_data: mapping of instance options
+        :param security_groups: list of security group ids to
+                                attach to the instance
+
+        :retval Returns a mapping of the instance information
+                that has just been created
+
+        """
+        instance_ref = self.db.instance_create(context, instance_data)
+        inst_id = instance_ref['id']
+
+        elevated = context.elevated()
+        for security_group_id in security_groups:
+            self.db.instance_add_security_group(elevated,
+                                                inst_id,
+                                                security_group_id)
+        return instance_ref
+
+    def update_instance(self, context, instance_id, instance_data):
+        """Updates the instance in the datastore
+
+        :param context: The security context
+        :param instance_data: mapping of instance options
+
+        :retval None
+
+        """
+        self.db.instance_update(context, instance_id, instance_data)
+
     @defer.inlineCallbacks
     @exception.wrap_exception
     def run_instance(self, context, instance_id, **_kwargs):
diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py
index 209d6e51f..74fd0fdc8 100644
--- a/nova/db/sqlalchemy/api.py
+++ b/nova/db/sqlalchemy/api.py
@@ -235,8 +235,7 @@ def service_get_by_args(context, host, binary):
 @require_admin_context
 def service_create(context, values):
     service_ref = models.Service()
-    for (key, value) in values.iteritems():
-        service_ref[key] = value
+    service_ref.update(values)
     service_ref.save()
     return service_ref
 
@@ -246,8 +245,7 @@ def service_update(context, service_id, values):
     session = get_session()
     with session.begin():
         service_ref = service_get(context, service_id, session=session)
-        for (key, value) in values.iteritems():
-            service_ref[key] = value
+        service_ref.update(values)
         service_ref.save(session=session)
 
 
@@ -278,8 +276,7 @@ def floating_ip_allocate_address(context, host, project_id):
 @require_context
 def floating_ip_create(context, values):
     floating_ip_ref = models.FloatingIp()
-    for (key, value) in values.iteritems():
-        floating_ip_ref[key] = value
+    floating_ip_ref.update(values)
     floating_ip_ref.save()
     return floating_ip_ref['address']
 
@@ -450,8 +447,7 @@ def fixed_ip_associate_pool(context, network_id, instance_id):
 @require_context
 def fixed_ip_create(_context, values):
     fixed_ip_ref = models.FixedIp()
-    for (key, value) in values.iteritems():
-        fixed_ip_ref[key] = value
+    fixed_ip_ref.update(values)
     fixed_ip_ref.save()
     return fixed_ip_ref['address']
 
@@ -520,8 +516,7 @@ def fixed_ip_update(context, address, values):
         fixed_ip_ref = fixed_ip_get_by_address(context,
                                                address,
                                                session=session)
-        for (key, value) in values.iteritems():
-            fixed_ip_ref[key] = value
+        fixed_ip_ref.update(values)
         fixed_ip_ref.save(session=session)
 
 
@@ -534,8 +529,7 @@ def fixed_ip_update(context, address, values):
 @require_context
 def instance_create(context, values):
     instance_ref = models.Instance()
-    for (key, value) in values.iteritems():
-        instance_ref[key] = value
+    instance_ref.update(values)
 
     session = get_session()
     with session.begin():
@@ -727,8 +721,7 @@ def instance_update(context, instance_id, values):
     session = get_session()
     with session.begin():
         instance_ref = instance_get(context, instance_id, session=session)
-        for (key, value) in values.iteritems():
-            instance_ref[key] = value
+        instance_ref.update(values)
         instance_ref.save(session=session)
 
 
@@ -750,8 +743,7 @@ def instance_add_security_group(context, instance_id, security_group_id):
 @require_context
 def key_pair_create(context, values):
     key_pair_ref = models.KeyPair()
-    for (key, value) in values.iteritems():
-        key_pair_ref[key] = value
+    key_pair_ref.update(values)
     key_pair_ref.save()
     return key_pair_ref
 
@@ -866,8 +858,7 @@ def network_count_reserved_ips(context, network_id):
 @require_admin_context
 def network_create_safe(context, values):
     network_ref = models.Network()
-    for (key, value) in values.iteritems():
-        network_ref[key] = value
+    network_ref.update(values)
     try:
         network_ref.save()
         return network_ref
@@ -976,8 +967,7 @@ def network_update(context, network_id, values):
     session = get_session()
     with session.begin():
         network_ref = network_get(context, network_id, session=session)
-        for (key, value) in values.iteritems():
-            network_ref[key] = value
+        network_ref.update(values)
         network_ref.save(session=session)
 
 
@@ -1027,8 +1017,7 @@ def export_device_count(context):
 @require_admin_context
 def export_device_create_safe(context, values):
     export_device_ref = models.ExportDevice()
-    for (key, value) in values.iteritems():
-        export_device_ref[key] = value
+    export_device_ref.update(values)
     try:
         export_device_ref.save()
         return export_device_ref
@@ -1054,8 +1043,7 @@ def auth_get_token(_context, token_hash):
 
 def auth_create_token(_context, token):
     tk = models.AuthToken()
-    for k,v in token.iteritems():
-        tk[k] = v
+    tk.update(token)
     tk.save()
     return tk
 
@@ -1081,8 +1069,7 @@ def quota_get(context, project_id, session=None):
 @require_admin_context
 def quota_create(context, values):
     quota_ref = models.Quota()
-    for (key, value) in values.iteritems():
-        quota_ref[key] = value
+    quota_ref.update(values)
     quota_ref.save()
     return quota_ref
 
@@ -1092,8 +1079,7 @@ def quota_update(context, project_id, values):
     session = get_session()
     with session.begin():
         quota_ref = quota_get(context, project_id, session=session)
-        for (key, value) in values.iteritems():
-            quota_ref[key] = value
+        quota_ref.update(values)
         quota_ref.save(session=session)
 
 
@@ -1141,8 +1127,7 @@ def volume_attached(context, volume_id, instance_id, mountpoint):
 @require_context
 def volume_create(context, values):
     volume_ref = models.Volume()
-    for (key, value) in values.iteritems():
-        volume_ref[key] = value
+    volume_ref.update(values)
 
     session = get_session()
     with session.begin():
@@ -1298,8 +1283,7 @@ def volume_update(context, volume_id, values):
     session = get_session()
     with session.begin():
         volume_ref = volume_get(context, volume_id, session=session)
-        for (key, value) in values.iteritems():
-            volume_ref[key] = value
+        volume_ref.update(values)
         volume_ref.save(session=session)
 
 
@@ -1392,8 +1376,7 @@ def security_group_create(context, values):
     # FIXME(devcamcar): Unless I do this, rules fails with lazy load exception
     # once save() is called.  This will get cleaned up in next orm pass.
     security_group_ref.rules
-    for (key, value) in values.iteritems():
-        security_group_ref[key] = value
+    security_group_ref.update(values)
     security_group_ref.save()
     return security_group_ref
 
@@ -1446,8 +1429,7 @@ def security_group_rule_get(context, security_group_rule_id, session=None):
 @require_context
 def security_group_rule_create(context, values):
     security_group_rule_ref = models.SecurityGroupIngressRule()
-    for (key, value) in values.iteritems():
-        security_group_rule_ref[key] = value
+    security_group_rule_ref.update(values)
     security_group_rule_ref.save()
     return security_group_rule_ref
 
@@ -1498,8 +1480,7 @@ def user_get_by_access_key(context, access_key, session=None):
 @require_admin_context
 def user_create(_context, values):
     user_ref = models.User()
-    for (key, value) in values.iteritems():
-        user_ref[key] = value
+    user_ref.update(values)
     user_ref.save()
     return user_ref
 
@@ -1527,8 +1508,7 @@ def user_get_all(context):
 
 def project_create(_context, values):
     project_ref = models.Project()
-    for (key, value) in values.iteritems():
-        project_ref[key] = value
+    project_ref.update(values)
     project_ref.save()
     return project_ref
 
@@ -1590,8 +1570,7 @@ def user_update(context, user_id, values):
     session = get_session()
     with session.begin():
         user_ref = user_get(context, user_id, session=session)
-        for (key, value) in values.iteritems():
-            user_ref[key] = value
+        user_ref.update(values)
         user_ref.save(session=session)
 
 
@@ -1599,8 +1578,7 @@ def project_update(context, project_id, values):
     session = get_session()
     with session.begin():
         project_ref = project_get(context, project_id, session=session)
-        for (key, value) in values.iteritems():
-            project_ref[key] = value
+        project_ref.update(values)
         project_ref.save(session=session)
 
 
diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py
index a63bca2b0..853c320e4 100644
--- a/nova/db/sqlalchemy/models.py
+++ b/nova/db/sqlalchemy/models.py
@@ -90,6 +90,16 @@ class NovaBase(object):
         n = self._i.next().name
         return n, getattr(self, n)
 
+    def update(self, values):
+        """Make the model object behave like a dict"""
+        for k, v in values.iteritems():
+            setattr(self, k, v)
+
+    def iteritems(self):
+        """Make the model object behave like a dict"""
+        return iter(self)
+
+
 # TODO(vish): Store images in the database instead of file system
 #class Image(BASE, NovaBase):
 #    """Represents an image in the datastore"""
diff --git a/nova/tests/api/openstack/fakes.py b/nova/tests/api/openstack/fakes.py
index 14170fbb2..f12c7b610 100644
--- a/nova/tests/api/openstack/fakes.py
+++ b/nova/tests/api/openstack/fakes.py
@@ -30,6 +30,7 @@ from nova import exception as exc
 import nova.api.openstack.auth
 from nova.image import service
 from nova.image.services import glance
+from nova.tests import fake_flags
 from nova.wsgi import Router
 
 
diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py
index d1ee533b6..f4a09fd97 100644
--- a/nova/tests/api/openstack/test_servers.py
+++ b/nova/tests/api/openstack/test_servers.py
@@ -92,9 +92,7 @@ class ServersTest(unittest.TestCase):
             pass 
 
         def instance_create(context, inst):
-            class Foo(object):
-                internal_id = 1
-            return Foo()
+            return {'id': 1, 'internal_id': 1}
 
         def fake_method(*args, **kwargs):
             pass