[Kimchi-devel] [PATCH v2 3/3] Add unit tests for remote-backed CD ROM updates.

Christy Perez christy at linux.vnet.ibm.com
Mon Aug 11 16:51:42 UTC 2014



On 08/10/2014 09:55 PM, Royce Lv wrote:
> On 2014年08月09日 03:34, Christy Perez wrote:
>> Adding tests to ensure that updating an existing CD ROM
>> can be done when using remote ISOs. This is to test for
>> regression for the libxml KeyError seen when even saving
>> the same CD ROM ISO as the update.
>>
>> Signed-off-by: Christy Perez <christy at linux.vnet.ibm.com>
>> ---
>>   tests/test_model.py | 55
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>   tests/utils.py      | 21 +++++++++++++++++++-
>>   2 files changed, 74 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/test_model.py b/tests/test_model.py
>> index 92b34b6..9821b42 100644
>> --- a/tests/test_model.py
>> +++ b/tests/test_model.py
>> @@ -285,7 +285,7 @@ def _attach_disk(expect_bus='virtio'):
>>               self.assertEquals(u'disk', disk_info['type'])
>>               inst.vmstorage_delete(vm_name, disk)
>>
>> -            # Specify pool and path at sametime will fail
>> +            # Specifying pool and path at same time will fail
>>               disk_args = {"type": "disk",
>>                            "pool": pool,
>>                            "vol": vol,
>> @@ -361,6 +361,33 @@ def test_vm_cdrom(self):
>>               self.assertRaises(InvalidParameter, inst.vmstorage_update,
>>                                 vm_name, cdrom_dev, {'path':
>> wrong_iso_path})
>>
>> +            # Make sure CD ROM still exists after failure
>> +            cd_info = inst.vmstorage_lookup(vm_name, cdrom_dev)
>> +            self.assertEquals(u'cdrom', cd_info['type'])
>> +            self.assertEquals(iso_path, cd_info['path'])
>> +
>> +            # update path of existing cd with valid remote ISO
>> +            # @TODO: This is currently not supported, so this
>> +            # test will fail.
>> +            valid_remote_iso_path = utils.get_remote_iso_path()
>> +            inst.vmstorage_update(vm_name, cdrom_dev,
>> +                {'path': valid_remote_iso_path})
>> +            cdrom_info = inst.vmstorage_lookup(vm_name, cdrom_dev)
>> +            self.assertEquals(valid_remote_iso_path,
>> +                    cdrom_info['path'],
>> +                    "This test will fail for now.\n"
>> +                    "There is no support for replacing a local ISO "
>> +                    "with a remote ISO." )
>> +
>> +            # update path of existing cd with invalid remote ISO
>> +            # @TODO: This is currently not supported, so this test
>> +            # is currently useless but will be valid later.
>> +            invalid_remote_iso_path =\
>> +                    'http://notreal.net/isos/fakeiso.iso'
>> +            self.assertRaises(OperationFailed, inst.vmstorages_update,
>> +                              vm_name, cdrom_dev,
>> +                              {'path': invalid_remote_iso_path})
>> +
>>               # update path of existing cd with existent iso of
>> shutoff vm
>>               inst.vmstorage_update(vm_name, cdrom_dev, {'path':
>> iso_path2})
>>               cdrom_info = inst.vmstorage_lookup(vm_name, cdrom_dev)
>> @@ -378,6 +405,13 @@ def test_vm_cdrom(self):
>>               self.assertEquals('', cdrom_info['path'])
>>               inst.vm_poweroff(vm_name)
>>
>> +            # Update path after eject. Local iso -> remote ISO.
>> +            inst.vmstorage_update(vm_name, cdrom_dev,
>> +                                  {'path': valid_remote_iso_path})
>> +            cdrom_info = inst.vmstorage_lookup(vm_name, cdrom_dev)
>> +            self.assertEquals(valid_remote_iso_path,
>> +                              cdrom_info['path'])
>> +
>>               # removing non existent cdrom
>>               self.assertRaises(NotFoundError, inst.vmstorage_delete,
>> vm_name,
>>                                 "fakedev")
>> @@ -387,6 +421,25 @@ def test_vm_cdrom(self):
>>               storage_list = inst.vmstorages_get_list(vm_name)
>>               self.assertEquals(prev_count, len(storage_list))
>>
>> +            # Create a new cdrom using a remote iso
>> +            cdrom_args = {"type": "cdrom",
>> +                          "path": valid_remote_iso_path}
>> +            cdrom_dev = inst.vmstorages_create(vm_name, cdrom_args)
>> +            storage_list = inst.vmstorages_get_list(vm_name)
>> +            self.assertEquals(prev_count + 1, len(storage_list))
KEEP
>> +
>> +            # Update remote-backed cdrom with the same ISO
>> +            inst.vmstorage_update(vm_name, cdrom_dev,
>> +                    {'path': valid_remote_iso_path})
>> +            cdrom_info = inst.vmstorage_lookup(vm_name, cdrom_dev)
>> +            self.assertEquals(valid_remote_iso_path,
>> +                              cdrom_info['path'])
KEEP
>> +
>> +            # Update remote-backed cdrom with invalid ISO
>> +            self.assertRaises(OperationFailed, inst.vmstorages_update,
>> +                              vm_name, cdrom_dev,
>> +                              {'path': invalid_remote_iso_path})
>> +
> I suggest we just retain the vmstorage_create testcases, and when update
> is fixed, we can add these test, because if make check fails I'm not
> sure build can continue in some auto build platform as we have already
> been added to fedora repo.
I'm okay with that. I'd like to keep the two I marked KEEP in since one
adds a remote CDROM and the other tests what this patch was fixing.

I'll send a v3 with this change.

>>       @unittest.skipUnless(utils.running_as_root(), 'Must be run as
>> root')
>>       def test_vm_storage_provisioning(self):
>>           inst = model.Model(objstore_loc=self.tmp_store)
>> diff --git a/tests/utils.py b/tests/utils.py
>> index 30c9cae..a5c0941 100644
>> --- a/tests/utils.py
>> +++ b/tests/utils.py
>> @@ -21,19 +21,20 @@
>>   import base64
>>   import cherrypy
>>   import httplib
>> +import json
>>   import os
>>   import socket
>>   import sys
>>   import threading
>>   import unittest
>>
>> -
>>   from contextlib import closing
>>   from lxml import etree
>>
>>
>>   import kimchi.mockmodel
>>   import kimchi.server
>> +from kimchi.config import paths
>>   from kimchi.exception import OperationFailed
>>
>>   _ports = {}
>> @@ -146,6 +147,24 @@ def request(host, port, path, data=None,
>> method='GET', headers=None):
>>       conn = httplib.HTTPSConnection(host, port)
>>       return _request(conn, path, data, method, headers)
>>
>> +def get_remote_iso_path():
>> +    """
>> +    Get a remote iso with the right arch from the distro files shipped
>> +    with kimchi.
>> +    """
>> +    host_arch = os.uname()[4]
>> +    remote_path = ''
>> +    with open(os.path.join(paths.conf_dir, 'distros.d', 'fedora.json'))\
>> +        as fedora_isos:
>> +        # Get a list of dicts
>> +        json_isos_list = json.load(fedora_isos)
>> +        for iso in json_isos_list:
>> +            if (iso.get('os_arch')) == host_arch:
>> +                remote_path = iso.get('path')
>> +                break
>> +
>> +    print(remote_path)
> I like this approach! But we need to delete this.
Oops! I thought I got all of those out. :)
>> +    return remote_path
>>
>>   def patch_auth(sudo=True):
>>       """
> 




More information about the Kimchi-devel mailing list