Fixed instance model associations to host (node) and added association to ip

3 files changed, 95 insertions, 96 deletions

diff --git a/nova/compute/model.py b/nova/compute/model.py
index 212830d3c..7dd130ca3 100644
--- a/nova/compute/model.py
+++ b/nova/compute/model.py
@@ -41,9 +41,6 @@ True
 """
 
 import datetime
-import logging
-import time
-import redis
 import uuid
 
 from nova import datastore
@@ -72,19 +69,22 @@ class InstanceDirectory(object):
         for instance_id in datastore.Redis.instance().smembers('project:%s:instances' % project):
             yield Instance(instance_id)
 
-    def by_node(self, node_id):
+    @datastore.absorb_connection_error
+    def by_node(self, node):
         """returns a list of instances for a node"""
+        for instance_id in datastore.Redis.instance().smembers('node:%s:instances' % node):
+            yield Instance(instance_id)
 
-        for instance in self.all:
-            if instance['node_name'] == node_id:
-                yield instance
-
-    def by_ip(self, ip_address):
+    def by_ip(self, 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
+        # NOTE(vish): The ip association should be just a single value, but
+        #             to maintain consistency it is using the standard
+        #             association and the ugly method for retrieving
+        #             the first item in the set below.
+        result = datastore.Redis.instance().smembers('ip:%s:instances' % ip)
+        if not result:
+            return None
+        return Instance(list(result)[0])
 
     def by_volume(self, volume_id):
         """returns the instance a volume is attached to"""
@@ -122,7 +122,8 @@ class Instance(datastore.BasicModel):
                 'instance_id': self.instance_id,
                 'node_name': 'unassigned',
                 'project_id': 'unassigned',
-                'user_id': 'unassigned'}
+                'user_id': 'unassigned',
+                'private_dns_name': 'unassigned'}
 
     @property
     def identifier(self):
@@ -148,19 +149,22 @@ class Instance(datastore.BasicModel):
         """Call into superclass to save object, then save associations"""
         # NOTE(todd): doesn't track migration between projects/nodes,
         #             it just adds the first one
-        should_update_project = self.is_new_record()
-        should_update_node = self.is_new_record()
+        is_new = self.is_new_record()
+        node_set = (self.state['node_name'] != 'unassigned' and
+                    self.initial_state['node_name'] == 'unassigned')
         success = super(Instance, self).save()
-        if success and should_update_project:
+        if success and is_new:
             self.associate_with("project", self.project)
-        if success and should_update_node:
-            self.associate_with("node", self['node_name'])
+            self.associate_with("ip", self.state['private_dns_name'])
+        if success and node_set:
+            self.associate_with("node", self.state['node_name'])
         return True
 
     def destroy(self):
         """Destroy associations, then destroy the object"""
         self.unassociate_with("project", self.project)
-        self.unassociate_with("node", self['node_name'])
+        self.unassociate_with("node", self.state['node_name'])
+        self.unassociate_with("ip", self.state['private_dns_name'])
         return super(Instance, self).destroy()
 
 class Host(datastore.BasicModel):
diff --git a/nova/datastore.py b/nova/datastore.py
index 9c2592334..51ef7a758 100644
--- a/nova/datastore.py
+++ b/nova/datastore.py
@@ -90,13 +90,15 @@ class BasicModel(object):
 
     @absorb_connection_error
     def __init__(self):
-        self.initial_state = {}
-        self.state = Redis.instance().hgetall(self.__redis_key)
-        if self.state:
-            self.initial_state = self.state
+        state = Redis.instance().hgetall(self.__redis_key)
+        if state:
+            self.initial_state = state
+            self.state = dict(self.initial_state)
         else:
+            self.initial_state = {}
             self.state = self.default_state()
 
+
     def default_state(self):
         """You probably want to define this in your subclass"""
         return {}
@@ -239,7 +241,7 @@ class BasicModel(object):
         for key, val in self.state.iteritems():
             Redis.instance().hset(self.__redis_key, key, val)
         self.add_to_index()
-        self.initial_state = self.state
+        self.initial_state = dict(self.state)
         return True
 
     @absorb_connection_error
diff --git a/nova/tests/model_unittest.py b/nova/tests/model_unittest.py
index 6825cfe2a..dc2441c24 100644
--- a/nova/tests/model_unittest.py
+++ b/nova/tests/model_unittest.py
@@ -19,9 +19,7 @@
 from datetime import datetime, timedelta
 import logging
 import time
-from twisted.internet import defer
 
-from nova import exception
 from nova import flags
 from nova import test
 from nova import utils
@@ -49,9 +47,9 @@ class ModelTestCase(test.TrialTestCase):
         inst['user_id'] = 'fake'
         inst['project_id'] = 'fake'
         inst['instance_type'] = 'm1.tiny'
-        inst['node_name'] = FLAGS.node_name
         inst['mac_address'] = utils.generate_mac()
         inst['ami_launch_index'] = 0
+        inst['private_dns_name'] = '10.0.0.1'
         inst.save()
         return inst
 
@@ -71,118 +69,126 @@ class ModelTestCase(test.TrialTestCase):
         session_token.save()
         return session_token
 
-    @defer.inlineCallbacks
     def test_create_instance(self):
         """store with create_instace, then test that a load finds it"""
-        instance = yield self.create_instance()
-        old = yield model.Instance(instance.identifier)
+        instance = self.create_instance()
+        old = model.Instance(instance.identifier)
         self.assertFalse(old.is_new_record())
 
-    @defer.inlineCallbacks
     def test_delete_instance(self):
         """create, then destroy, then make sure loads a new record"""
-        instance = yield self.create_instance()
-        yield instance.destroy()
-        newinst = yield model.Instance('i-test')
+        instance = self.create_instance()
+        instance.destroy()
+        newinst = model.Instance('i-test')
         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"""
-        instance = yield self.create_instance()
+        """create, then check that it is listed in global set"""
+        instance = self.create_instance()
         found = False
         for x in model.InstanceDirectory().all:
             if x.identifier == 'i-test':
                 found = True
         self.assert_(found)
 
-    @defer.inlineCallbacks
     def test_instance_associates_project(self):
         """create, then check that it is listed for the project"""
-        instance = yield self.create_instance()
+        instance = self.create_instance()
         found = False
         for x in model.InstanceDirectory().by_project(instance.project):
             if x.identifier == 'i-test':
                 found = True
         self.assert_(found)
 
-    @defer.inlineCallbacks
+    def test_instance_associates_ip(self):
+        """create, then check that it is listed for the ip"""
+        instance = self.create_instance()
+        found = False
+        x = model.InstanceDirectory().by_ip(instance['private_dns_name'])
+        self.assertEqual(x.identifier, 'i-test')
+
+    def test_instance_associates_node(self):
+        """create, then check that it is listed for the node_name"""
+        instance = self.create_instance()
+        found = False
+        for x in model.InstanceDirectory().by_node(FLAGS.node_name):
+            if x.identifier == 'i-test':
+                found = True
+        self.assertFalse(found)
+        instance['node_name'] = 'test_node'
+        instance.save()
+        for x in model.InstanceDirectory().by_node('test_node'):
+            if x.identifier == 'i-test':
+                found = True
+        self.assert_(found)
+
+
     def test_host_class_finds_hosts(self):
-        host = yield self.create_host()
+        host = self.create_host()
         self.assertEqual('testhost', model.Host.lookup('testhost').identifier)
 
-    @defer.inlineCallbacks
     def test_host_class_doesnt_find_missing_hosts(self):
-        rv = yield model.Host.lookup('woahnelly')
+        rv = model.Host.lookup('woahnelly')
         self.assertEqual(None, rv)
 
-    @defer.inlineCallbacks
     def test_create_host(self):
         """store with create_host, then test that a load finds it"""
-        host = yield self.create_host()
-        old = yield model.Host(host.identifier)
+        host = self.create_host()
+        old = model.Host(host.identifier)
         self.assertFalse(old.is_new_record())
 
-    @defer.inlineCallbacks
     def test_delete_host(self):
         """create, then destroy, then make sure loads a new record"""
-        instance = yield self.create_host()
-        yield instance.destroy()
-        newinst = yield model.Host('testhost')
+        instance = self.create_host()
+        instance.destroy()
+        newinst = model.Host('testhost')
         self.assertTrue(newinst.is_new_record())
 
-    @defer.inlineCallbacks
     def test_host_added_to_set(self):
         """create, then check that it is included in list"""
-        instance = yield self.create_host()
+        instance = self.create_host()
         found = False
         for x in model.Host.all():
             if x.identifier == 'testhost':
                 found = True
         self.assert_(found)
 
-    @defer.inlineCallbacks
     def test_create_daemon_two_args(self):
         """create a daemon with two arguments"""
-        d = yield self.create_daemon()
+        d = 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 = 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 = 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 = 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.Daemon('testhost', 'nova-testdaemon')
+        instance = self.create_daemon()
+        instance.destroy()
+        newinst = model.Daemon('testhost', 'nova-testdaemon')
         self.assertTrue(newinst.is_new_record())
 
-    @defer.inlineCallbacks
     def test_daemon_heartbeat(self):
         """Create a daemon, sleep, heartbeat, check for update"""
-        d = yield self.create_daemon()
+        d = self.create_daemon()
         ts = d['updated_at']
         time.sleep(2)
         d.heartbeat()
@@ -190,70 +196,62 @@ class ModelTestCase(test.TrialTestCase):
         ts2 = d2['updated_at']
         self.assert_(ts2 > ts)
 
-    @defer.inlineCallbacks
     def test_daemon_added_to_set(self):
         """create, then check that it is included in list"""
-        instance = yield self.create_daemon()
+        instance = self.create_daemon()
         found = False
         for x in model.Daemon.all():
             if x.identifier == 'testhost:nova-testdaemon':
                 found = True
         self.assert_(found)
 
-    @defer.inlineCallbacks
     def test_daemon_associates_host(self):
         """create, then check that it is listed for the host"""
-        instance = yield self.create_daemon()
+        instance = self.create_daemon()
         found = False
         for x in model.Daemon.by_host('testhost'):
             if x.identifier == 'testhost:nova-testdaemon':
                 found = True
         self.assertTrue(found)
 
-    @defer.inlineCallbacks
     def test_create_session_token(self):
         """create"""
-        d = yield self.create_session_token()
+        d = self.create_session_token()
         d = model.SessionToken(d.token)
         self.assertFalse(d.is_new_record())
 
-    @defer.inlineCallbacks
     def test_delete_session_token(self):
         """create, then destroy, then make sure loads a new record"""
-        instance = yield self.create_session_token()
-        yield instance.destroy()
-        newinst = yield model.SessionToken(instance.token)
+        instance = self.create_session_token()
+        instance.destroy()
+        newinst = model.SessionToken(instance.token)
         self.assertTrue(newinst.is_new_record())
 
-    @defer.inlineCallbacks
     def test_session_token_added_to_set(self):
         """create, then check that it is included in list"""
-        instance = yield self.create_session_token()
+        instance = self.create_session_token()
         found = False
         for x in model.SessionToken.all():
             if x.identifier == instance.token:
                 found = True
         self.assert_(found)
 
-    @defer.inlineCallbacks
     def test_session_token_associates_user(self):
         """create, then check that it is listed for the user"""
-        instance = yield self.create_session_token()
+        instance = self.create_session_token()
         found = False
         for x in model.SessionToken.associated_to('user', 'testuser'):
             if x.identifier == instance.identifier:
                 found = True
         self.assertTrue(found)
 
-    @defer.inlineCallbacks
     def test_session_token_generation(self):
-        instance = yield model.SessionToken.generate('username', 'TokenType')
+        instance = model.SessionToken.generate('username', 'TokenType')
         self.assertFalse(instance.is_new_record())
 
-    @defer.inlineCallbacks
     def test_find_generated_session_token(self):
-        instance = yield model.SessionToken.generate('username', 'TokenType')
-        found = yield model.SessionToken.lookup(instance.identifier)
+        instance = model.SessionToken.generate('username', 'TokenType')
+        found = model.SessionToken.lookup(instance.identifier)
         self.assert_(found)
 
     def test_update_session_token_expiry(self):
@@ -264,34 +262,29 @@ class ModelTestCase(test.TrialTestCase):
         expiry = utils.parse_isotime(instance['expiry'])
         self.assert_(expiry > datetime.utcnow())
 
-    @defer.inlineCallbacks
     def test_session_token_lookup_when_expired(self):
-        instance = yield model.SessionToken.generate("testuser")
+        instance = model.SessionToken.generate("testuser")
         instance['expiry'] = datetime.utcnow().strftime(utils.TIME_FORMAT)
         instance.save()
         inst = model.SessionToken.lookup(instance.identifier)
         self.assertFalse(inst)
 
-    @defer.inlineCallbacks
     def test_session_token_lookup_when_not_expired(self):
-        instance = yield model.SessionToken.generate("testuser")
+        instance = model.SessionToken.generate("testuser")
         inst = model.SessionToken.lookup(instance.identifier)
         self.assert_(inst)
 
-    @defer.inlineCallbacks
     def test_session_token_is_expired_when_expired(self):
-        instance = yield model.SessionToken.generate("testuser")
+        instance = model.SessionToken.generate("testuser")
         instance['expiry'] = datetime.utcnow().strftime(utils.TIME_FORMAT)
         self.assert_(instance.is_expired())
 
-    @defer.inlineCallbacks
     def test_session_token_is_expired_when_not_expired(self):
-        instance = yield model.SessionToken.generate("testuser")
+        instance = model.SessionToken.generate("testuser")
         self.assertFalse(instance.is_expired())
 
-    @defer.inlineCallbacks
     def test_session_token_ttl(self):
-        instance = yield model.SessionToken.generate("testuser")
+        instance = model.SessionToken.generate("testuser")
         now = datetime.utcnow()
         delta = timedelta(hours=1)
         instance['expiry'] = (now + delta).strftime(utils.TIME_FORMAT)