Merge lp:~jason-koelker/nova/lp852079 into lp:~hudson-openstack/nova/trunk

[Jason Kölker]

Remove vestigial db call for fixed_ips.

Remove vestigial db call for fixed_ips.

[Devin Carlen (devcamcar)]

Just to make sure I'm reading this right, all the needed info returned by instance_get_fixed_addresses is now available in nwinfo, yes?

[Jason Kölker (jason-koelker)]

In this instance, yes. Since the only thing pre_live_migrate did is check that fixed_ips existed.

[Vish Ishaya (vishvananda)]

lgtm.

review: Approve

[Brian Waldon (bcwaldon)]

Great cleanup.

review: Approve

=== modified file 'nova/compute/manager.py'
--- nova/compute/manager.py	2011-09-12 19:39:13 +0000
+++ nova/compute/manager.py	2011-09-16 18:57:24 +0000
@@ -1388,11 +1388,6 @@
         instance_ref = self.db.instance_get(context, instance_id)
         hostname = instance_ref['hostname']
 
-        # Getting fixed ips
-        fixed_ips = self.db.instance_get_fixed_addresses(context, instance_id)
-        if not fixed_ips:
-            raise exception.FixedIpNotFoundForInstance(instance_id=instance_id)
-
         # If any volume is mounted, prepare here.
         if not instance_ref['volumes']:
             LOG.info(_("%s has no volume."), hostname)
@@ -1408,6 +1403,11 @@
         # Retry operation is necessary because continuously request comes,
         # concorrent request occurs to iptables, then it complains.
         network_info = self._get_instance_nw_info(context, instance_ref)
+
+        fixed_ips = [nw_info[1]['ips'] for nw_info in network_info]
+        if not fixed_ips:
+            raise exception.FixedIpNotFoundForInstance(instance_id=instance_id)
+
         max_retry = FLAGS.live_migration_retry_count
         for cnt in range(max_retry):
             try:

=== modified file 'nova/tests/test_compute.py'
--- nova/tests/test_compute.py	2011-09-09 18:05:33 +0000
+++ nova/tests/test_compute.py	2011-09-16 18:57:24 +0000
@@ -647,7 +647,6 @@
 
         dbmock = self.mox.CreateMock(db)
         dbmock.instance_get(c, i_id).AndReturn(instance_ref)
-        dbmock.instance_get_fixed_addresses(c, i_id).AndReturn(None)
 
         self.compute.db = dbmock
         self.mox.ReplayAll()
@@ -657,6 +656,9 @@
 
     def test_pre_live_migration_instance_has_volume(self):
         """Confirm setup_compute_volume is called when volume is mounted."""
+        def fake_nw_info(*args, **kwargs):
+            return [(0, {'ips':['dummy']})]
+
         i_ref = self._get_dummy_instance()
         c = context.get_admin_context()
 
@@ -666,13 +668,13 @@
         drivermock = self.mox.CreateMock(self.compute_driver)
 
         dbmock.instance_get(c, i_ref['id']).AndReturn(i_ref)
-        dbmock.instance_get_fixed_addresses(c, i_ref['id']).AndReturn('dummy')
         for i in range(len(i_ref['volumes'])):
             vid = i_ref['volumes'][i]['id']
             volmock.setup_compute_volume(c, vid).InAnyOrder('g1')
-        drivermock.plug_vifs(i_ref, [])
-        drivermock.ensure_filtering_rules_for_instance(i_ref, [])
+        drivermock.plug_vifs(i_ref, fake_nw_info())
+        drivermock.ensure_filtering_rules_for_instance(i_ref, fake_nw_info())
 
+        self.stubs.Set(self.compute, '_get_instance_nw_info', fake_nw_info)
         self.compute.db = dbmock
         self.compute.volume_manager = volmock
         self.compute.driver = drivermock
@@ -683,6 +685,9 @@
 
     def test_pre_live_migration_instance_has_no_volume(self):
         """Confirm log meg when instance doesn't mount any volumes."""
+        def fake_nw_info(*args, **kwargs):
+            return [(0, {'ips':['dummy']})]
+
         i_ref = self._get_dummy_instance()
         i_ref['volumes'] = []
         c = context.get_admin_context()
@@ -692,12 +697,12 @@
         drivermock = self.mox.CreateMock(self.compute_driver)
 
         dbmock.instance_get(c, i_ref['id']).AndReturn(i_ref)
-        dbmock.instance_get_fixed_addresses(c, i_ref['id']).AndReturn('dummy')
         self.mox.StubOutWithMock(compute_manager.LOG, 'info')
         compute_manager.LOG.info(_("%s has no volume."), i_ref['hostname'])
-        drivermock.plug_vifs(i_ref, [])
-        drivermock.ensure_filtering_rules_for_instance(i_ref, [])
+        drivermock.plug_vifs(i_ref, fake_nw_info())
+        drivermock.ensure_filtering_rules_for_instance(i_ref, fake_nw_info())
 
+        self.stubs.Set(self.compute, '_get_instance_nw_info', fake_nw_info)
         self.compute.db = dbmock
         self.compute.driver = drivermock
 
@@ -711,6 +716,8 @@
         It retries and raise exception when timeout exceeded.
 
         """
+        def fake_nw_info(*args, **kwargs):
+            return [(0, {'ips':['dummy']})]
 
         i_ref = self._get_dummy_instance()
         c = context.get_admin_context()
@@ -722,13 +729,13 @@
         drivermock = self.mox.CreateMock(self.compute_driver)
 
         dbmock.instance_get(c, i_ref['id']).AndReturn(i_ref)
-        dbmock.instance_get_fixed_addresses(c, i_ref['id']).AndReturn('dummy')
         for i in range(len(i_ref['volumes'])):
             volmock.setup_compute_volume(c, i_ref['volumes'][i]['id'])
         for i in range(FLAGS.live_migration_retry_count):
-            drivermock.plug_vifs(i_ref, []).\
+            drivermock.plug_vifs(i_ref, fake_nw_info()).\
                 AndRaise(exception.ProcessExecutionError())
 
+        self.stubs.Set(self.compute, '_get_instance_nw_info', fake_nw_info)
         self.compute.db = dbmock
         self.compute.network_manager = netmock
         self.compute.volume_manager = volmock