Fixed xenapi tests

Gave up on clever things with map stored as string in xenstore. Used ast.liteeral_eval instead.

Changed instance ID and names in tests from in to string (1 => '1')
This simplified VMOps._get_vm_opaqueref

Changed VMOps._get_vm_opaqueref as references returned by fake xenapi do not start with "OpaqueRef:" prefix.


Fixed PEP8 Errors

6 files changed, 32 insertions, 58 deletions

diff --git a/nova/tests/db/fakes.py b/nova/tests/db/fakes.py
index b939f99a0..7aa72e4a3 100644
--- a/nova/tests/db/fakes.py
+++ b/nova/tests/db/fakes.py
@@ -36,8 +36,6 @@ class FakeModel(object):
         if key in self.values:
             return self.values[key]
         else:
-            print "Key:%s" %key
-            print "Values:%s" %self.values
             raise NotImplementedError()
 
 
@@ -74,7 +72,7 @@ def stub_out_db_instance_api(stubs):
 
 def stub_out_db_network_api(stubs, injected=True):
     """Stubs out the db API for retrieving networks"""
-    
+
     network_fields = {
         'id': 'test',
         'bridge': 'xenbr0',
@@ -83,12 +81,12 @@ def stub_out_db_network_api(stubs, injected=True):
         'gateway': '10.0.0.1',
         'broadcast': '10.0.0.255',
         'dns': '10.0.0.2',
-        'ra_server': None,        
+        'ra_server': None,
         'injected': injected}
-    
+
     fixed_ip_fields = {
-        'address':'10.0.0.3',
-        'network_id':'test'}
+        'address': '10.0.0.3',
+        'network_id': 'test'}
 
     def fake_network_get_by_instance(context, instance_id):
         return FakeModel(network_fields)
@@ -113,4 +111,3 @@ def stub_out_db_network_api(stubs, injected=True):
               fake_network_get_all_by_instance)
     stubs.Set(db, 'fixed_ip_get_all_by_instance',
         fake_fixed_ip_get_all_by_instance)
-    
\ No newline at end of file
diff --git a/nova/tests/test_xenapi.py b/nova/tests/test_xenapi.py
index 10a1b6c11..a3d3371a1 100644
--- a/nova/tests/test_xenapi.py
+++ b/nova/tests/test_xenapi.py
@@ -21,6 +21,7 @@ Test suite for XenAPI
 import os
 import re
 import stubout
+import ast
 
 from nova import db
 from nova import context
@@ -204,7 +205,7 @@ class XenAPIVMTestCase(test.TestCase):
                 if not vm_rec["is_control_domain"]:
                     vm_labels.append(vm_rec["name_label"])
 
-            self.assertEquals(vm_labels, [1])
+            self.assertEquals(vm_labels, ['1'])
 
         def ensure_vbd_was_torn_down():
             vbd_labels = []
@@ -212,7 +213,7 @@ class XenAPIVMTestCase(test.TestCase):
                 vbd_rec = xenapi_fake.get_record('VBD', vbd_ref)
                 vbd_labels.append(vbd_rec["vm_name_label"])
 
-            self.assertEquals(vbd_labels, [1])
+            self.assertEquals(vbd_labels, ['1'])
 
         def ensure_vdi_was_torn_down():
             for vdi_ref in xenapi_fake.get_all('VDI'):
@@ -229,10 +230,10 @@ class XenAPIVMTestCase(test.TestCase):
 
     def check_vm_record(self, conn, check_injection=False):
         instances = conn.list_instances()
-        self.assertEquals(instances, [1])
+        self.assertEquals(instances, ['1'])
 
         # Get Nova record for VM
-        vm_info = conn.get_info(1)
+        vm_info = conn.get_info('1')
 
         # Get XenAPI record for VM
         vms = [(ref, rec) for ref, rec
@@ -262,27 +263,24 @@ class XenAPIVMTestCase(test.TestCase):
         if check_injection:
             xenstore_data = xenapi_fake.VM_get_xenstore_data(vm_ref)
             key = 'vm-data/networking/aabbccddeeff'
-            LOG.debug("Xenstore data: %s",xenstore_data)
-            xenstore_value=xenstore_data[key]
-            #tcpip_data = dict([(k, v)
-            #    for k, v in xenstore_value.iteritems()
-            #    if k.startswith(key_prefix)])
-            #LOG.debug("tcpip data: %s",tcpip_data)
-            #self.assertEquals(tcpip_data['label'],'test_network')
-            #self.assertEquals(tcpip_data, {
-            #    'label': 'test_network',
-            #    'broadcast': '10.0.0.255',
-            #    'ips': [{'ip': '10.0.0.3', 'netmask':'255.255.255.0', 'enabled':'1'}],
-            #    'mac': 'aa:bb:cc:dd:ee:ff',
-            #    'dns': ['10.0.0.2'],
-            #    'gateway': '10.0.0.1'})
+            xenstore_value = xenstore_data[key]
+            tcpip_data = ast.literal_eval(xenstore_value)
+            self.assertEquals(tcpip_data, {
+                'label': 'test_network',
+                'broadcast': '10.0.0.255',
+                'ips': [{'ip': '10.0.0.3',
+                         'netmask':'255.255.255.0',
+                         'enabled':'1'}],
+                'mac': 'aa:bb:cc:dd:ee:ff',
+                'dns': ['10.0.0.2'],
+                'gateway': '10.0.0.1'})
 
     def _test_spawn(self, image_id, kernel_id, ramdisk_id,
         instance_type="m1.large", check_injection=False):
 
         stubs.stubout_session(self.stubs, stubs.FakeSessionForVMTests)
-        values = {'name': 1,
-                  'id': 1,
+        values = {'name': "1",
+                  'id': "1",
                   'project_id': self.project.id,
                   'user_id': self.user.id,
                   'image_id': image_id,
@@ -331,9 +329,7 @@ class XenAPIVMTestCase(test.TestCase):
 
             # Find the start of eth0 configuration and check it
             index = config.index('auto eth0')
-            LOG.debug("CONFIG")
-            LOG.debug(config)
-            
+
             self.assertEquals(config[index + 1:index + 8], [
                 'iface eth0 inet static',
                 'address 10.0.0.3',
@@ -409,8 +405,8 @@ class XenAPIVMTestCase(test.TestCase):
     def _create_instance(self):
         """Creates and spawns a test instance"""
         values = {
-            'name': 1,
-            'id': 1,
+            'name': '1',
+            'id': '1',
             'project_id': self.project.id,
             'user_id': self.user.id,
             'image_id': 1,
diff --git a/nova/tests/xenapi/stubs.py b/nova/tests/xenapi/stubs.py
index 551d326a4..2392a97b3 100644
--- a/nova/tests/xenapi/stubs.py
+++ b/nova/tests/xenapi/stubs.py
@@ -191,7 +191,7 @@ class FakeSessionForVMTests(fake.SessionBase):
 
     def VM_add_to_xenstore_data(self, session_ref, vm_ref, key, value):
         fake.VM_add_to_xenstore_data(vm_ref, key, value)
-    
+
     def VM_remove_from_xenstore_data(self, session_ref, vm_ref, key):
         fake.VM_remove_from_xenstore_data(vm_ref, key)
 
diff --git a/nova/virt/xenapi/fake.py b/nova/virt/xenapi/fake.py
index 664cfbd79..7aa82211d 100644
--- a/nova/virt/xenapi/fake.py
+++ b/nova/virt/xenapi/fake.py
@@ -156,11 +156,11 @@ def VM_get_xenstore_data(vm_ref):
 def VM_remove_from_xenstore_data(vm_ref, key):
     db_ref = _db_content['VM'][vm_ref]
     if not 'xenstore_data' in db_ref:
-        return 
+        return
     db_ref['xenstore_data'][key] = None
 
+
 def VM_add_to_xenstore_data(vm_ref, key, value):
-    LOG.debug("ADDING TO XENSTORE DATA %s %s",key,value)
     db_ref = _db_content['VM'][vm_ref]
     if not 'xenstore_data' in db_ref:
         db_ref['xenstore_data'] = {}
@@ -510,9 +510,7 @@ class SessionBase(object):
 
     def _get_by_field(self, recs, k, v, return_singleton):
         result = []
-        LOG.debug("_get_by_field!!!! - %d", return_singleton)
         for ref, rec in recs.iteritems():
-            LOG.debug("k:%s,rec[k]:%s,v:%s",k,rec.get(k),v)
             if rec.get(k) == v:
                 result.append(ref)
 
diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py
index a01bab8de..510261b15 100644
--- a/nova/virt/xenapi/vm_utils.py
+++ b/nova/virt/xenapi/vm_utils.py
@@ -409,10 +409,8 @@ class VMHelper(HelperBase):
     @classmethod
     def lookup(cls, session, i):
         """Look the instance i up, and returns it if available"""
-        LOG.debug("Entering lookup for instance:%s",str(i))
         vms = session.get_xenapi().VM.get_by_name_label(i)
         n = len(vms)
-        LOG.debug("n:%d",n)        
         if n == 0:
             return None
         elif n > 1:
@@ -451,20 +449,17 @@ class VMHelper(HelperBase):
         # As mounting the image VDI is expensive, we only want do do it once,
         # if at all, so determine whether it's required first, and then do
         # everything
-        LOG.debug("Running preconfigure_instance")
         mount_required = False
         key, net = disk.get_injectables(instance)
         if key is not None or net is not None:
             mount_required = True
 
-        LOG.debug("Mount_required:%s", str(mount_required))
         if mount_required:
 
             def _mounted_processing(device):
                 """Callback which runs with the image VDI attached"""
 
                 dev_path = '/dev/' + device + '1'  # NB: Partition 1 hardcoded
-                LOG.debug("Device path:%s", dev_path)
                 tmpdir = tempfile.mkdtemp()
                 try:
                     # Mount only Linux filesystems, to avoid disturbing
@@ -473,7 +468,6 @@ class VMHelper(HelperBase):
                         out, err = utils.execute(
                             'sudo mount -t ext2,ext3 "%s" "%s"' %
                             (dev_path, tmpdir))
-                        LOG.debug("filesystem mounted")
                     except exception.ProcessExecutionError as e:
                         err = str(e)
                     if err:
@@ -506,7 +500,6 @@ class VMHelper(HelperBase):
                                         'installed in this image'))
                                 LOG.info(_('Manipulating interface files '
                                          'directly'))
-                                LOG.debug("Going to inject data in filesystem")
                                 disk.inject_data_into_fs(tmpdir, key, net,
                                     utils.execute)
                         finally:
@@ -748,7 +741,6 @@ def vbd_unplug_with_retry(session, vbd):
     # FIXME(sirp): We can use LoopingCall here w/o blocking sleep()
     while True:
         try:
-            LOG.debug("About to unplug VBD")
             session.get_xenapi().VBD.unplug(vbd)
             LOG.debug(_('VBD.unplug successful first time.'))
             return
diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py
index c137d4931..a2f3a8f09 100644
--- a/nova/virt/xenapi/vmops.py
+++ b/nova/virt/xenapi/vmops.py
@@ -112,12 +112,10 @@ class VMOps(object):
 
         # Alter the image before VM start for, e.g. network injection
         #TODO(salvatore-orlando): do this only if flag is true
-        LOG.debug("About to run preconfigure_instance")
         if FLAGS.xenapi_inject_image:
             VMHelper.preconfigure_instance(self._session, instance, vdi_ref)
 
         # inject_network_info and create vifs
-        LOG.debug("About to run inject_network_info")
         networks = self.inject_network_info(instance)
         self.create_vifs(instance, networks)
 
@@ -192,19 +190,12 @@ class VMOps(object):
             # Not a string; must be an ID or a vm instance
             if isinstance(instance_or_vm, (int, long)):
                 ctx = context.get_admin_context()
-                try:
-                    instance_obj = db.instance_get(ctx, instance_or_vm)
-                    instance_name = instance_obj.name
-                except exception.NotFound:
-                    # The unit tests screw this up, as they use an integer for
-                    # the vm name. I'd fix that up, but that's a matter for
-                    # another bug report. So for now, just try with the passed
-                    # value
-                    instance_name = instance_or_vm
+                instance_obj = db.instance_get(ctx, instance_or_vm)
+                instance_name = instance_obj.name
             else:
                 instance_name = instance_or_vm.name
         #fake xenapi does not use OpaqueRef as a prefix
-        #when running tests we will always end up here 
+        #when running tests we will always end up here
         vm = VMHelper.lookup(self._session, instance_name)
         if vm is None:
             if FLAGS.xenapi_connection_url == 'test_url':