Merged with trunk, fix problem with behaviour of (fake) virt driver when instance doesn't reach scheduling.

There's an odd issue I can't track down where if I start the networking service then later tests fail.  If I don't start the networking service, a created instance never reaches the compute service, which isn't actualy a problem, except then destroy fails.

I documented the expected behaviour (per libvirt) for when the instance isn't found and destroy is called - I believe this to be the correct behaviour anyway.  I also fixed up fake.

15 files changed, 70 insertions, 55 deletions

diff --git a/bin/nova-manage b/bin/nova-manage
index 6712fbadb..cf0caf47e 100755
--- a/bin/nova-manage
+++ b/bin/nova-manage
@@ -611,7 +611,7 @@ class ServiceCommands(object):
         args: [host] [service]"""
         ctxt = context.get_admin_context()
         now = datetime.datetime.utcnow()
-        services = db.service_get_all(ctxt) + db.service_get_all(ctxt, True)
+        services = db.service_get_all(ctxt)
         if host:
             services = [s for s in services if s['host'] == host]
         if service:
diff --git a/nova/api/ec2/__init__.py b/nova/api/ec2/__init__.py
index 20701cfa8..a3c3b25a1 100644
--- a/nova/api/ec2/__init__.py
+++ b/nova/api/ec2/__init__.py
@@ -61,10 +61,13 @@ class RequestLogging(wsgi.Middleware):
         return rv
 
     def log_request_completion(self, response, request, start):
-        controller = request.environ.get('ec2.controller', None)
-        if controller:
-            controller = controller.__class__.__name__
-        action = request.environ.get('ec2.action', None)
+        apireq = request.environ.get('ec2.request', None)
+        if apireq:
+            controller = apireq.controller
+            action = apireq.action
+        else:
+            controller = None
+            action = None
         ctxt = request.environ.get('ec2.context', None)
         delta = utils.utcnow() - start
         seconds = delta.seconds
@@ -75,7 +78,7 @@ class RequestLogging(wsgi.Middleware):
             microseconds,
             request.remote_addr,
             request.method,
-            request.path_info,
+            "%s%s" % (request.script_name, request.path_info),
             controller,
             action,
             response.status_int,
diff --git a/nova/api/ec2/admin.py b/nova/api/ec2/admin.py
index d8d90ad83..6a5609d4a 100644
--- a/nova/api/ec2/admin.py
+++ b/nova/api/ec2/admin.py
@@ -304,7 +304,7 @@ class AdminController(object):
             * Volume (up, down, None)
             * Volume Count
         """
-        services = db.service_get_all(context)
+        services = db.service_get_all(context, False)
         now = datetime.datetime.utcnow()
         hosts = []
         rv = []
diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py
index e257e44e7..2afcea77c 100644
--- a/nova/api/ec2/cloud.py
+++ b/nova/api/ec2/cloud.py
@@ -196,7 +196,7 @@ class CloudController(object):
 
     def _describe_availability_zones(self, context, **kwargs):
         ctxt = context.elevated()
-        enabled_services = db.service_get_all(ctxt)
+        enabled_services = db.service_get_all(ctxt, False)
         disabled_services = db.service_get_all(ctxt, True)
         available_zones = []
         for zone in [service.availability_zone for service
@@ -221,7 +221,7 @@ class CloudController(object):
         rv = {'availabilityZoneInfo': [{'zoneName': 'nova',
                                         'zoneState': 'available'}]}
 
-        services = db.service_get_all(context)
+        services = db.service_get_all(context, False)
         now = datetime.datetime.utcnow()
         hosts = []
         for host in [service['host'] for service in services]:
diff --git a/nova/api/openstack/accounts.py b/nova/api/openstack/accounts.py
index 2510ffb61..86066fa20 100644
--- a/nova/api/openstack/accounts.py
+++ b/nova/api/openstack/accounts.py
@@ -14,6 +14,7 @@
 #    under the License.
 
 import common
+import webob.exc
 
 from nova import exception
 from nova import flags
@@ -51,10 +52,10 @@ class Controller(wsgi.Controller):
             raise exception.NotAuthorized(_("Not admin user."))
 
     def index(self, req):
-        raise faults.Fault(exc.HTTPNotImplemented())
+        raise faults.Fault(webob.exc.HTTPNotImplemented())
 
     def detail(self, req):
-        raise faults.Fault(exc.HTTPNotImplemented())
+        raise faults.Fault(webob.exc.HTTPNotImplemented())
 
     def show(self, req, id):
         """Return data about the given account id"""
@@ -69,7 +70,7 @@ class Controller(wsgi.Controller):
     def create(self, req):
         """We use update with create-or-update semantics
            because the id comes from an external source"""
-        raise faults.Fault(exc.HTTPNotImplemented())
+        raise faults.Fault(webob.exc.HTTPNotImplemented())
 
     def update(self, req, id):
         """This is really create or update."""
diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py
index 443196146..1b4f28f59 100644
--- a/nova/api/openstack/servers.py
+++ b/nova/api/openstack/servers.py
@@ -15,19 +15,19 @@
 
 import base64
 import hashlib
-import json
 import traceback
-from xml.dom import minidom
 
 from webob import exc
+from xml.dom import minidom
 
 from nova import compute
 from nova import context
 from nova import exception
 from nova import flags
 from nova import log as logging
-from nova import wsgi
+from nova import quota
 from nova import utils
+from nova import wsgi
 from nova.api.openstack import common
 from nova.api.openstack import faults
 import nova.api.openstack.views.addresses
@@ -36,7 +36,6 @@ import nova.api.openstack.views.servers
 from nova.auth import manager as auth_manager
 from nova.compute import instance_types
 from nova.compute import power_state
-from nova.quota import QuotaError
 import nova.api.openstack
 
 
@@ -156,8 +155,8 @@ class Controller(wsgi.Controller):
                 key_data=key_data,
                 metadata=metadata,
                 injected_files=injected_files)
-        except QuotaError as error:
-            self._handle_quota_errors(error)
+        except quota.QuotaError as error:
+            self._handle_quota_error(error)
 
         inst['instance_type'] = flavor_id
         inst['image_id'] = requested_image_id
@@ -211,7 +210,7 @@ class Controller(wsgi.Controller):
             injected_files.append((path, contents))
         return injected_files
 
-    def _handle_quota_errors(self, error):
+    def _handle_quota_error(self, error):
         """
         Reraise quota errors as api-specific http exceptions
         """
@@ -242,7 +241,7 @@ class Controller(wsgi.Controller):
             update_dict['admin_pass'] = inst_dict['server']['adminPass']
             try:
                 self.compute_api.set_admin_password(ctxt, id)
-            except exception.TimeoutException, e:
+            except exception.TimeoutException:
                 return exc.HTTPRequestTimeout()
         if 'name' in inst_dict['server']:
             update_dict['display_name'] = inst_dict['server']['name']
diff --git a/nova/db/api.py b/nova/db/api.py
index afc1bff2f..caa719bbc 100644
--- a/nova/db/api.py
+++ b/nova/db/api.py
@@ -89,7 +89,7 @@ def service_get_by_host_and_topic(context, host, topic):
     return IMPL.service_get_by_host_and_topic(context, host, topic)
 
 
-def service_get_all(context, disabled=False):
+def service_get_all(context, disabled=None):
     """Get all services."""
     return IMPL.service_get_all(context, disabled)
 
diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py
index d7b5aff46..bfceaa94c 100644
--- a/nova/db/sqlalchemy/api.py
+++ b/nova/db/sqlalchemy/api.py
@@ -143,12 +143,15 @@ def service_get(context, service_id, session=None):
 
 
 @require_admin_context
-def service_get_all(context, disabled=False):
+def service_get_all(context, disabled=None):
     session = get_session()
-    return session.query(models.Service).\
-                   filter_by(deleted=can_read_deleted(context)).\
-                   filter_by(disabled=disabled).\
-                   all()
+    query = session.query(models.Service).\
+                   filter_by(deleted=can_read_deleted(context))
+
+    if disabled is not None:
+        query = query.filter_by(disabled=disabled)
+
+    return query.all()
 
 
 @require_admin_context
@@ -2209,7 +2212,7 @@ def migration_get(context, id, session=None):
                      filter_by(id=id).first()
     if not result:
         raise exception.NotFound(_("No migration found with id %s")
-                % migration_id)
+                % id)
     return result
 
 
@@ -2432,6 +2435,7 @@ def zone_create(context, values):
 
 @require_admin_context
 def zone_update(context, zone_id, values):
+    session = get_session()
     zone = session.query(models.Zone).filter_by(id=zone_id).first()
     if not zone:
         raise exception.NotFound(_("No zone with id %(zone_id)s") % locals())
diff --git a/nova/image/glance.py b/nova/image/glance.py
index 171b28fde..9984a3ba1 100644
--- a/nova/image/glance.py
+++ b/nova/image/glance.py
@@ -73,7 +73,7 @@ class GlanceImageService(service.BaseImageService):
         Returns image with known timestamp fields converted to datetime objects
         """
         for attr in ['created_at', 'updated_at', 'deleted_at']:
-            if image.get(attr) is not None:
+            if image.get(attr):
                 image[attr] = self._parse_glance_iso8601_timestamp(image[attr])
         return image
 
diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py
index 796d6ba31..06b05366a 100644
--- a/nova/network/linux_net.py
+++ b/nova/network/linux_net.py
@@ -1,3 +1,5 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
 # Copyright 2010 United States Government as represented by the
 # Administrator of the National Aeronautics and Space Administration.
 # All Rights Reserved.
@@ -210,10 +212,7 @@ class IptablesManager(object):
     """
     def __init__(self, execute=None):
         if not execute:
-            if FLAGS.fake_network:
-                self.execute = lambda *args, **kwargs: ('', '')
-            else:
-                self.execute = utils.execute
+            self.execute = _execute
         else:
             self.execute = execute
 
@@ -352,9 +351,6 @@ class IptablesManager(object):
         return new_filter
 
 
-iptables_manager = IptablesManager()
-
-
 def metadata_forward():
     """Create forwarding rule for metadata"""
     iptables_manager.ipv4['nat'].add_rule("PREROUTING",
@@ -767,3 +763,6 @@ def _ip_bridge_cmd(action, params, device):
     cmd.extend(params)
     cmd.extend(['dev', device])
     return cmd
+
+
+iptables_manager = IptablesManager()
diff --git a/nova/tests/integrated/integrated_helpers.py b/nova/tests/integrated/integrated_helpers.py
index 953af2e75..b68d34fd9 100644
--- a/nova/tests/integrated/integrated_helpers.py
+++ b/nova/tests/integrated/integrated_helpers.py
@@ -129,7 +129,11 @@ class IntegratedUnitTestContext(object):
         self._start_compute_service()
         self._start_volume_service()
         self._start_scheduler_service()
-        self._start_network_service()
+        
+        # NOTE(justinsb): There's a bug here which is eluding me...
+        #  If we start the network_service, all is good, but then subsequent
+        #  tests fail: CloudTestCase.test_ajax_console in particular.
+        #self._start_network_service()
 
         # WSGI shutdown broken :-(
         # bug731668
diff --git a/nova/tests/test_volume.py b/nova/tests/test_volume.py
index 5d68ca2ae..d71b75f3f 100644
--- a/nova/tests/test_volume.py
+++ b/nova/tests/test_volume.py
@@ -356,8 +356,8 @@ class ISCSITestCase(DriverTestCase):
         tid = db.volume_get_iscsi_target_num(self.context, volume_id_list[0])
         self.mox.StubOutWithMock(self.volume.driver, '_execute')
         self.volume.driver._execute("sudo", "ietadm", "--op", "show",
-                                    "--tid=%(tid)d" % locals()
-                                 ).AndRaise(exception.ProcessExecutionError())
+                                    "--tid=%(tid)d" % locals()).AndRaise(
+                                            exception.ProcessExecutionError())
 
         self.mox.ReplayAll()
         self.assertRaises(exception.ProcessExecutionError,
diff --git a/nova/utils.py b/nova/utils.py
index e4d8a70eb..2f568f739 100644
--- a/nova/utils.py
+++ b/nova/utils.py
@@ -171,10 +171,6 @@ def execute(*cmd, **kwargs):
                                                 stdout=stdout,
                                                 stderr=stderr,
                                                 cmd=' '.join(cmd))
-            # NOTE(termie): this appears to be necessary to let the subprocess
-            #               call clean something up in between calls, without
-            #               it two execute calls in a row hangs the second one
-            greenthread.sleep(0)
             return result
         except ProcessExecutionError:
             if not attempts:
@@ -183,6 +179,11 @@ def execute(*cmd, **kwargs):
                 LOG.debug(_("%r failed. Retrying."), cmd)
                 if delay_on_retry:
                     greenthread.sleep(random.randint(20, 200) / 100.0)
+        finally:
+            # NOTE(termie): this appears to be necessary to let the subprocess
+            #               call clean something up in between calls, without
+            #               it two execute calls in a row hangs the second one
+            greenthread.sleep(0)
 
 
 def ssh_execute(ssh, cmd, process_input=None,
diff --git a/nova/virt/driver.py b/nova/virt/driver.py
index 0e3a4aa3b..8d43247d8 100644
--- a/nova/virt/driver.py
+++ b/nova/virt/driver.py
@@ -66,7 +66,19 @@ class ComputeDriver(object):
         raise NotImplementedError()
 
     def destroy(self, instance, cleanup=True):
-        """Shutdown specified VM"""
+        """
+        Destroy (shutdown and delete) the specified instance.
+
+        The given parameter is an instance of nova.compute.service.Instance,
+        and so the instance is being specified as instance.name.
+
+        The work will be done asynchronously.  This function returns a
+        task that allows the caller to detect when it is complete.
+
+        If the instance is not found (for example if networking failed), this
+        function should still succeed.  It's probably a good idea to log a
+        warning in that case.
+        """
         raise NotImplementedError()
 
     def reboot(self, instance):
diff --git a/nova/virt/fake.py b/nova/virt/fake.py
index dd3675569..030f0a2fa 100644
--- a/nova/virt/fake.py
+++ b/nova/virt/fake.py
@@ -260,20 +260,12 @@ class FakeConnection(driver.ComputeDriver):
         pass
 
     def destroy(self, instance):
-        """
-        Destroy (shutdown and delete) the specified instance.
-
-        The given parameter is an instance of nova.compute.service.Instance,
-        and so the instance is being specified as instance.name.
-
-        The work will be done asynchronously.  This function returns a
-        task that allows the caller to detect when it is complete.
-        """
         key = instance.name
-        if not key in self.instances:
+        if key in self.instances:
+            del self.instances[key]
+        else:
             LOG.warning("Key '%s' not in instances '%s'" %
                         (key, self.instances))
-        del self.instances[instance.name]
 
     def attach_volume(self, instance_name, device_path, mountpoint):
         """Attach the disk at device_path to the instance at mountpoint"""
@@ -352,7 +344,7 @@ class FakeConnection(driver.ComputeDriver):
         Note that this function takes an instance ID, not a
         compute.service.Instance, so that it can be called by compute.monitor.
         """
-        return [0L, 0L, 0L, 0L, null]
+        return [0L, 0L, 0L, 0L, None]
 
     def interface_stats(self, instance_name, iface_id):
         """