Fixes based on code review 27001.

8 files changed, 170 insertions, 179 deletions

diff --git a/nova/adminclient.py b/nova/adminclient.py
index 19dba28aa..480e907c9 100644
--- a/nova/adminclient.py
+++ b/nova/adminclient.py
@@ -46,9 +46,11 @@ class UserInfo(object):
     def __repr__(self):
         return 'UserInfo:%s' % self.username
 
+    # this is needed by the sax parser, so ignore the ugly name
     def startElement(self, name, attrs, connection):
         return None
 
+    # this is needed by the sax parser, so ignore the ugly name
     def endElement(self, name, value, connection):
         if name == 'username':
             self.username = str(value)
@@ -78,9 +80,11 @@ class HostInfo(object):
     def __repr__(self):
         return 'Host:%s' % self.hostname
 
+    # this is needed by the sax parser, so ignore the ugly name
     def startElement(self, name, attrs, connection):
         return None
 
+    # this is needed by the sax parser, so ignore the ugly name
     def endElement(self, name, value, connection):
         setattr(self, name, value)
 
diff --git a/nova/auth/users.py b/nova/auth/users.py
index 1105fea82..b9d77f86f 100644
--- a/nova/auth/users.py
+++ b/nova/auth/users.py
@@ -328,7 +328,8 @@ class UserManager(object):
 
         user = self.get_user_from_access_key(access_key)
         if user == None:
-            raise exception.NotFound('No user found for access key %s' % access_key)
+            raise exception.NotFound('No user found for access key %s' %
+                                     access_key)
         if project_name is '':
             project_name = user.name
 
diff --git a/nova/compute/model.py b/nova/compute/model.py
index ddcb120e4..a4f0eac4d 100644
--- a/nova/compute/model.py
+++ b/nova/compute/model.py
@@ -56,9 +56,11 @@ from nova import utils
 
 FLAGS = flags.FLAGS
 
+
 class ConnectionError(exception.Error):
     pass
 
+
 def absorb_connection_error(fn):
     def _wrapper(*args, **kwargs):
         try:
@@ -67,12 +69,13 @@ def absorb_connection_error(fn):
             raise ConnectionError(str(ce))
     return _wrapper
 
+
 # TODO(ja): singleton instance of the directory
 class InstanceDirectory(object):
-    """an api for interacting with the global state of instances """
+    """an api for interacting with the global state of instances"""
 
     def get(self, instance_id):
-        """ returns an instance object for a given id """
+        """returns an instance object for a given id"""
         return Instance(instance_id)
 
     def __getitem__(self, item):
@@ -80,26 +83,26 @@ class InstanceDirectory(object):
 
     @absorb_connection_error
     def by_project(self, project):
-        """ returns a list of instance objects for a project """
+        """returns a list of instance objects for a project"""
         for instance_id in datastore.Redis.instance().smembers('project:%s:instances' % project):
             yield Instance(instance_id)
 
     def by_node(self, node_id):
-        """ returns a list of instances for a node """
+        """returns a list of instances for a node"""
 
         for instance in self.all:
             if instance['node_name'] == node_id:
                 yield instance
 
     def by_ip(self, ip_address):
-        """ returns an instance object that is using the IP """
+        """returns an instance object that is using the IP"""
         for instance in self.all:
             if instance['private_dns_name'] == ip_address:
                 return instance
         return None
 
     def by_volume(self, volume_id):
-        """ returns the instance a volume is attached to """
+        """returns the instance a volume is attached to"""
         pass
 
     @absorb_connection_error
@@ -109,12 +112,12 @@ class InstanceDirectory(object):
     @property
     @absorb_connection_error
     def all(self):
-        """ returns a list of all instances """
+        """returns a list of all instances"""
         for instance_id in datastore.Redis.instance().smembers('instances'):
             yield Instance(instance_id)
 
     def new(self):
-        """ returns an empty Instance object, with ID """
+        """returns an empty Instance object, with ID"""
         instance_id = utils.generate_uid('i')
         return self.get(instance_id)
 
@@ -129,13 +132,13 @@ class BasicModel(object):
             self.state = self.default_state()
 
     def default_state(self):
-        """ You probably want to define this in your subclass """
+        """You probably want to define this in your subclass"""
         return {}
 
     @classmethod
     def lookup(cls, identifier):
         rv = cls(identifier)
-        if rv.new_record():
+        if rv.is_new_record():
             return None
         else:
             return rv
@@ -143,7 +146,7 @@ class BasicModel(object):
     @classmethod
     @absorb_connection_error
     def all(cls):
-        """ yields all objects in the store """
+        """yields all objects in the store"""
         redis_set = cls._redis_set_name(cls.__name__)
         for identifier in datastore.Redis.instance().smembers(redis_set):
             yield cls(identifier)
@@ -162,15 +165,13 @@ class BasicModel(object):
 
     @classmethod
     def _redis_association_name(cls, foreign_type, foreign_id):
-        return cls._redis_set_name(
-                        "%s:%s:%s" % 
-                        (foreign_type, foreign_id, cls.__name__)
-                    )
+        return cls._redis_set_name("%s:%s:%s" % 
+                                   (foreign_type, foreign_id, cls.__name__))
 
     @property
     def identifier(self):
-        """ You DEFINITELY want to define this in your subclass """
-        raise Exception("Your sublcass should define identifier")
+        """You DEFINITELY want to define this in your subclass"""
+        raise NotImplementedError("Your sublcass should define identifier")
 
     @property
     def __redis_key(self):
@@ -205,10 +206,10 @@ class BasicModel(object):
         return self.state[item]
 
     def __delitem__(self, item):
-        """ We don't support this """
+        """We don't support this"""
         raise Exception("Silly monkey, models NEED all their properties.")
 
-    def new_record(self):
+    def is_new_record(self):
         return self.initial_state == {}
 
     @absorb_connection_error
@@ -222,22 +223,23 @@ class BasicModel(object):
         datastore.Redis.instance().srem(set_name, self.identifier)
 
     @absorb_connection_error
+    def remove_from_index(self):
+        set_name = self.__class__._redis_set_name(self.__class__.__name__)
+        datastore.Redis.instance().srem(set_name, self.identifier)
+
+    @absorb_connection_error
     def associate_with(self, foreign_type, foreign_id):
         # note the extra 's' on the end is for plurality
         # to match the old data without requiring a migration of any sort
         self.add_associated_model_to_its_set(foreign_type, foreign_id)
-        redis_set = self.__class__._redis_association_name(
-                        foreign_type,
-                        foreign_id
-                    )
+        redis_set = self.__class__._redis_association_name(foreign_type,
+                                                           foreign_id)
         datastore.Redis.instance().sadd(redis_set, self.identifier)
 
     @absorb_connection_error
     def unassociate_with(self, foreign_type, foreign_id):
-        redis_set = self.__class__._redis_association_name(
-                        foreign_type,
-                        foreign_id
-                    )
+        redis_set = self.__class__._redis_association_name(foreign_type,
+                                                           foreign_id)
         datastore.Redis.instance().srem(redis_set, self.identifier)
 
     def add_associated_model_to_its_set(self, my_type, my_id):
@@ -248,10 +250,9 @@ class BasicModel(object):
             my_inst = my_class(my_id)
             my_inst.save()
         else:
-            logging.warning(
-                "no model class for %s when building association from %s" %
-                (klsname, self)
-            )
+            logging.warning("no model class for %s when building"
+                            " association from %s",
+                            klsname, self)
 
     @absorb_connection_error
     def save(self):
@@ -276,32 +277,29 @@ class BasicModel(object):
         deletes all related records from datastore.
         does NOT do anything to running libvirt state.
         """
-        logging.info(
-            "Destroying datamodel for %s %s",
-            (self.__class__.__name__, self.identifier)
-        )
+        logging.info("Destroying datamodel for %s %s",
+                     self.__class__.__name__, self.identifier)
         datastore.Redis.instance().delete(self.__redis_key)
+        self.remove_from_index()
         return True
 
 
 class Instance(BasicModel):
-    """ Wrapper around stored properties of an instance """
+    """Wrapper around stored properties of an instance"""
 
     def __init__(self, instance_id):
-        """ loads an instance from the datastore if exists """
+        """loads an instance from the datastore if exists"""
         # set instance data before super call since it uses default_state
         self.instance_id = instance_id
         super(Instance, self).__init__()
 
     def default_state(self):
-        return {
-            'state': 0,
-            'state_description': 'pending',
-            'instance_id': self.instance_id,
-            'node_name': 'unassigned',
-            'project_id': 'unassigned',
-            'user_id': 'unassigned'
-        }
+        return {'state': 0,
+                'state_description': 'pending',
+                'instance_id': self.instance_id,
+                'node_name': 'unassigned',
+                'project_id': 'unassigned',
+                'user_id': 'unassigned'}
 
     @property
     def identifier(self):
@@ -315,58 +313,55 @@ class Instance(BasicModel):
 
     @property
     def volumes(self):
-        """ returns a list of attached volumes """
+        """returns a list of attached volumes"""
         pass
 
     @property
     def reservation(self):
-        """ Returns a reservation object """
+        """Returns a reservation object"""
         pass
 
     def save(self):
-        """ Call into superclass to save object, then save associations """
-        # XXX: doesn't track migration between projects, just adds the first one
-        should_update_project = self.new_record()
+        """Call into superclass to save object, then save associations"""
+        # NOTE(todd): doesn't track migration between projects,
+        #             it just adds the first one
+        should_update_project = self.is_new_record()
         success = super(Instance, self).save()
         if success and should_update_project:
             self.associate_with("project", self.project)
         return True
 
     def destroy(self):
-        """ Destroy associations, then destroy the object """
+        """Destroy associations, then destroy the object"""
         self.unassociate_with("project", self.project)
         return super(Instance, self).destroy()
 
 class Host(BasicModel):
     """
-    A Host is the base machine that runs many virtualized Instance.
-    Hosts are usually controlled vi nova.compute.node.Node, this model
-    just stores stats about a host in redis.
+    A Host is the machine where a Daemon is running.
     """
 
     def __init__(self, hostname):
-        """ loads an instance from the datastore if exists """
+        """loads an instance from the datastore if exists"""
         # set instance data before super call since it uses default_state
         self.hostname = hostname
         super(Host, self).__init__()
 
     def default_state(self):
-        return {
-            "hostname":     self.hostname
-        }
+        return {"hostname": self.hostname}
 
     @property
     def identifier(self):
         return self.hostname
 
 
-class Worker(BasicModel):
+class Daemon(BasicModel):
     """
-    A Worker is a job (compute, api, network, ...) that runs on a host.
+    A Daemon is a job (compute, api, network, ...) that runs on a host.
     """
 
     def __init__(self, host_or_combined, binpath=None):
-        """ loads an instance from the datastore if exists """
+        """loads an instance from the datastore if exists"""
         # set instance data before super call since it uses default_state
         # since loading from datastore expects a combined key that
         # is equivilent to identifier, we need to expect that, while
@@ -377,34 +372,34 @@ class Worker(BasicModel):
             self.binary = binpath
         else:
             self.hostname, self.binary = host_or_combined.split(":")
-        super(Worker, self).__init__()
+        super(Daemon, self).__init__()
 
     def default_state(self):
-        return {
-            "hostname":     self.hostname,
-            "binary":       self.binary,
-            "updated_at":   utils.timestamp()
-        }
+        return {"hostname":     self.hostname,
+                "binary":       self.binary,
+                "updated_at":   utils.isotime()
+                }
 
     @property
     def identifier(self):
         return "%s:%s" % (self.hostname, self.binary)
 
     def save(self):
-        """ Call into superclass to save object, then save associations """
-        # XXX: doesn't clear out from host list after crash, termination, etc
-        success = super(Worker, self).save()
+        """Call into superclass to save object, then save associations"""
+        # NOTE(todd): this makes no attempt to destroy itsself,
+        #             so after termination a record w/ old timestmap remains
+        success = super(Daemon, self).save()
         if success:
             self.associate_with("host", self.hostname)
         return True
 
     def destroy(self):
-        """ Destroy associations, then destroy the object """
+        """Destroy associations, then destroy the object"""
         self.unassociate_with("host", self.hostname)
-        return super(Worker, self).destroy()
+        return super(Daemon, self).destroy()
 
     def heartbeat(self):
-        self['updated_at'] = utils.timestamp()
+        self['updated_at'] = utils.isotime()
         self.save()
         return True
 
diff --git a/nova/compute/node.py b/nova/compute/node.py
index b0f6173c9..faf1ddd15 100644
--- a/nova/compute/node.py
+++ b/nova/compute/node.py
@@ -142,9 +142,11 @@ class Node(object, service.Service):
         return retval
 
     @defer.inlineCallbacks
-    def report_state(self, hostname, worker):
+    def report_state(self, nodename, daemon):
+        # TODO(termie): Termie has an idea for wrapping this connection failure
+        #               pattern to be more elegant.  -todd
         try:
-            record = model.Worker(hostname, worker)
+            record = model.Daemon(nodename, daemon)
             record.heartbeat()
             if getattr(self, "model_disconnected", False):
                 self.model_disconnected = False
diff --git a/nova/endpoint/admin.py b/nova/endpoint/admin.py
index 839cd9ad4..4471a4ef4 100644
--- a/nova/endpoint/admin.py
+++ b/nova/endpoint/admin.py
@@ -25,7 +25,7 @@ Admin API controller, exposed through http via the api worker.
 import base64
 
 def user_dict(user, base64_file=None):
-    """ Convert the user object to a result dict """
+    """Convert the user object to a result dict"""
     if user:
         return {
             'username': user.id,
@@ -37,16 +37,16 @@ def user_dict(user, base64_file=None):
         return {}
 
 def host_dict(host):
-    """ Convert a host model object to a result dict """
+    """Convert a host model object to a result dict"""
     if host:
         return host.state
     else:
         return {}
 
 def admin_only(target):
-    """ Decorator for admin-only API calls """
+    """Decorator for admin-only API calls"""
     def wrapper(*args, **kwargs):
-        """ Internal wrapper method for admin-only API calls """
+        """Internal wrapper method for admin-only API calls"""
         context = args[1]
         if context.user.is_admin():
             return target(*args, **kwargs)
@@ -71,19 +71,18 @@ class AdminController(object):
 
     @admin_only
     def describe_user(self, _context, name, **_kwargs):
-        """ Returns user data, including access and secret keys. """
+        """Returns user data, including access and secret keys."""
         return user_dict(self.user_manager.get_user(name))
 
     @admin_only
     def describe_users(self, _context, **_kwargs):
-        """ Returns all users - should be changed to deal with a list. """
+        """Returns all users - should be changed to deal with a list."""
         return {'userSet':
             [user_dict(u) for u in self.user_manager.get_users()] }
 
     @admin_only
     def register_user(self, _context, name, **_kwargs):
-        """ Creates a new user, and returns generated credentials.
-        """
+        """Creates a new user, and returns generated credentials."""
         return user_dict(self.user_manager.create_user(name))
 
     @admin_only
@@ -118,11 +117,9 @@ class AdminController(object):
             * DHCP servers running
             * Iptables / bridges
         """
-        return {'hostSet':
-            [host_dict(h) for h in self.host_manager.all()] }
+        return {'hostSet': [host_dict(h) for h in self.host_manager.all()]}
 
     @admin_only
     def describe_host(self, _context, name, **_kwargs):
-        """Returns status info for single node.
-        """
+        """Returns status info for single node."""
         return host_dict(self.host_manager.lookup(name))
diff --git a/nova/tests/model_unittest.py b/nova/tests/model_unittest.py
index 4eecdfeeb..23e2f9e73 100644
--- a/nova/tests/model_unittest.py
+++ b/nova/tests/model_unittest.py
@@ -26,12 +26,12 @@ from nova import utils
 from nova.compute import model
 from nova.compute import node
 
+
 FLAGS = flags.FLAGS
 
 
 class ModelTestCase(test.TrialTestCase):
     def setUp(self):
-        logging.getLogger().setLevel(logging.DEBUG)
         super(ModelTestCase, self).setUp()
         self.flags(fake_libvirt=True,
                    fake_storage=True,
@@ -40,7 +40,7 @@ class ModelTestCase(test.TrialTestCase):
     def tearDown(self):
         model.Instance('i-test').destroy()
         model.Host('testhost').destroy()
-        model.Worker('testhost', 'nova-testworker').destroy()
+        model.Daemon('testhost', 'nova-testdaemon').destroy()
 
     def create_instance(self):
         inst = model.Instance('i-test')
@@ -60,45 +60,45 @@ class ModelTestCase(test.TrialTestCase):
         host.save()
         return host
 
-    def create_worker(self):
-        worker = model.Worker('testhost', 'nova-testworker')
-        worker.save()
-        return worker
+    def create_daemon(self):
+        daemon = model.Daemon('testhost', 'nova-testdaemon')
+        daemon.save()
+        return daemon
 
     @defer.inlineCallbacks
     def test_create_instance(self):
-        """ store with create_instace, then test that a load finds it """
+        """store with create_instace, then test that a load finds it"""
         instance = yield self.create_instance()
         old = yield model.Instance(instance.identifier)
-        self.assertEqual(False, old.new_record())
+        self.assertFalse(old.is_new_record())
 
     @defer.inlineCallbacks
     def test_delete_instance(self):
-        """ create, then destroy, then make sure loads a new record """
+        """create, then destroy, then make sure loads a new record"""
         instance = yield self.create_instance()
         yield instance.destroy()
         newinst = yield model.Instance('i-test')
-        self.assertEqual(True, newinst.new_record())
+        self.assertTrue(newinst.is_new_record())
 
     @defer.inlineCallbacks
     def test_instance_added_to_set(self):
-        """ create, then check that it is listed for the project """
+        """create, then check that it is listed for the project"""
         instance = yield self.create_instance()
         found = False
         for x in model.InstanceDirectory().all:
             if x.identifier == 'i-test':
                 found = True
-        self.assertEqual(True, found)
+        self.assert_(found)
 
     @defer.inlineCallbacks
     def test_instance_associates_project(self):
-        """ create, then check that it is listed for the project """
+        """create, then check that it is listed for the project"""
         instance = yield self.create_instance()
         found = False
         for x in model.InstanceDirectory().by_project(instance.project):
             if x.identifier == 'i-test':
                 found = True
-        self.assertEqual(True, found)
+        self.assert_(found)
 
     @defer.inlineCallbacks
     def test_host_class_finds_hosts(self):
@@ -112,102 +112,94 @@ class ModelTestCase(test.TrialTestCase):
 
     @defer.inlineCallbacks
     def test_create_host(self):
-        """ store with create_host, then test that a load finds it """
+        """store with create_host, then test that a load finds it"""
         host = yield self.create_host()
         old = yield model.Host(host.identifier)
-        self.assertEqual(False, old.new_record())
+        self.assertFalse(old.is_new_record())
 
     @defer.inlineCallbacks
     def test_delete_host(self):
-        """ create, then destroy, then make sure loads a new record """
+        """create, then destroy, then make sure loads a new record"""
         instance = yield self.create_host()
         yield instance.destroy()
         newinst = yield model.Host('testhost')
-        self.assertEqual(True, newinst.new_record())
+        self.assertTrue(newinst.is_new_record())
 
     @defer.inlineCallbacks
     def test_host_added_to_set(self):
-        """ create, then check that it is included in list """
+        """create, then check that it is included in list"""
         instance = yield self.create_host()
         found = False
         for x in model.Host.all():
             if x.identifier == 'testhost':
                 found = True
-        self.assertEqual(True, found)
-
-    @defer.inlineCallbacks
-    def test_create_worker_two_args(self):
-        """ create a worker with two arguments """
-        w = yield self.create_worker()
-        self.assertEqual(
-            False,
-            model.Worker('testhost', 'nova-testworker').new_record()
-        )
-
-    @defer.inlineCallbacks
-    def test_create_worker_single_arg(self):
-        """ Create a worker using the combined host:bin format """
-        w = yield model.Worker("testhost:nova-testworker")
-        w.save()
-        self.assertEqual(
-            False,
-            model.Worker('testhost:nova-testworker').new_record()
-        )
-
-    @defer.inlineCallbacks
-    def test_equality_of_worker_single_and_double_args(self):
-        """ Create a worker using the combined host:bin arg, find with 2 """
-        w = yield model.Worker("testhost:nova-testworker")
-        w.save()
-        self.assertEqual(
-            False,
-            model.Worker('testhost', 'nova-testworker').new_record()
-        )
-
-    @defer.inlineCallbacks
-    def test_equality_worker_of_double_and_single_args(self):
-        """ Create a worker using the combined host:bin arg, find with 2 """
-        w = yield self.create_worker()
-        self.assertEqual(
-            False,
-            model.Worker('testhost:nova-testworker').new_record()
-        )
-
-    @defer.inlineCallbacks
-    def test_delete_worker(self):
-        """ create, then destroy, then make sure loads a new record """
-        instance = yield self.create_worker()
+        self.assert_(found)
+
+    @defer.inlineCallbacks
+    def test_create_daemon_two_args(self):
+        """create a daemon with two arguments"""
+        d = yield self.create_daemon()
+        d = model.Daemon('testhost', 'nova-testdaemon')
+        self.assertFalse(d.is_new_record())
+
+    @defer.inlineCallbacks
+    def test_create_daemon_single_arg(self):
+        """Create a daemon using the combined host:bin format"""
+        d = yield model.Daemon("testhost:nova-testdaemon")
+        d.save()
+        d = model.Daemon('testhost:nova-testdaemon')
+        self.assertFalse(d.is_new_record())
+
+    @defer.inlineCallbacks
+    def test_equality_of_daemon_single_and_double_args(self):
+        """Create a daemon using the combined host:bin arg, find with 2"""
+        d = yield model.Daemon("testhost:nova-testdaemon")
+        d.save()
+        d = model.Daemon('testhost', 'nova-testdaemon')
+        self.assertFalse(d.is_new_record())
+
+    @defer.inlineCallbacks
+    def test_equality_daemon_of_double_and_single_args(self):
+        """Create a daemon using the combined host:bin arg, find with 2"""
+        d = yield self.create_daemon()
+        d = model.Daemon('testhost:nova-testdaemon')
+        self.assertFalse(d.is_new_record())
+
+    @defer.inlineCallbacks
+    def test_delete_daemon(self):
+        """create, then destroy, then make sure loads a new record"""
+        instance = yield self.create_daemon()
         yield instance.destroy()
-        newinst = yield model.Worker('testhost', 'nova-testworker')
-        self.assertEqual(True, newinst.new_record())
+        newinst = yield model.Daemon('testhost', 'nova-testdaemon')
+        self.assertTrue(newinst.is_new_record())
 
     @defer.inlineCallbacks
-    def test_worker_heartbeat(self):
-        """ Create a worker, sleep, heartbeat, check for update """
-        w = yield self.create_worker()
-        ts = w['updated_at']
-        yield time.sleep(2)
-        w.heartbeat()
-        w2 = model.Worker('testhost', 'nova-testworker')
-        ts2 = w2['updated_at']
-        self.assertEqual(True, (ts2 > ts))
+    def test_daemon_heartbeat(self):
+        """Create a daemon, sleep, heartbeat, check for update"""
+        d = yield self.create_daemon()
+        ts = d['updated_at']
+        time.sleep(2)
+        d.heartbeat()
+        d2 = model.Daemon('testhost', 'nova-testdaemon')
+        ts2 = d2['updated_at']
+        self.assert_(ts2 > ts)
 
     @defer.inlineCallbacks
-    def test_worker_added_to_set(self):
-        """ create, then check that it is included in list """
-        instance = yield self.create_worker()
+    def test_daemon_added_to_set(self):
+        """create, then check that it is included in list"""
+        instance = yield self.create_daemon()
         found = False
-        for x in model.Worker.all():
-            if x.identifier == 'testhost:nova-testworker':
+        for x in model.Daemon.all():
+            if x.identifier == 'testhost:nova-testdaemon':
                 found = True
-        self.assertEqual(True, found)
+        self.assert_(found)
 
     @defer.inlineCallbacks
-    def test_worker_associates_host(self):
-        """ create, then check that it is listed for the host """
-        instance = yield self.create_worker()
+    def test_daemon_associates_host(self):
+        """create, then check that it is listed for the host"""
+        instance = yield self.create_daemon()
         found = False
-        for x in model.Worker.by_host('testhost'):
-            if x.identifier == 'testhost:nova-testworker':
+        for x in model.Daemon.by_host('testhost'):
+            if x.identifier == 'testhost:nova-testdaemon':
                 found = True
-        self.assertEqual(True, found)
+        self.assertTrue(found)
diff --git a/nova/utils.py b/nova/utils.py
index e445a8bc4..094de5d74 100644
--- a/nova/utils.py
+++ b/nova/utils.py
@@ -29,7 +29,7 @@ import os.path
 import inspect
 import subprocess
 import random
-import time
+from datetime import datetime
 
 from nova import flags
 
@@ -116,7 +116,7 @@ def get_my_ip():
     csock.close()
     return addr
 
-def timestamp(at=None):
+def isotime(at=None):
     if not at:
-        at = time.gmtime()
-    return time.strftime("%Y-%m-%dT%H:%M:%SZ", at)
+        at = datetime.utcnow()
+    return at.strftime("%Y-%m-%dT%H:%M:%SZ")
diff --git a/run_tests.py b/run_tests.py
index 7d5e74887..91e886c76 100644
--- a/run_tests.py
+++ b/run_tests.py
@@ -53,6 +53,7 @@ from nova.tests.access_unittest import *
 from nova.tests.api_unittest import *
 from nova.tests.cloud_unittest import *
 from nova.tests.keeper_unittest import *
+from nova.tests.model_unittest import *
 from nova.tests.network_unittest import *
 from nova.tests.node_unittest import *
 from nova.tests.objectstore_unittest import *
@@ -61,7 +62,6 @@ from nova.tests.storage_unittest import *
 from nova.tests.users_unittest import *
 from nova.tests.datastore_unittest import *
 from nova.tests.validator_unittest import *
-from nova.tests.model_unittest import *
 
 
 FLAGS = flags.FLAGS