Signed-off-by: Ramon Medeiros <ramonn at linux.vnet.ibm.com> --- utils.py | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/utils.py b/utils.py index f43f26e..0a3d02d 100644 --- a/utils.py +++ b/utils.py @@ -35,6 +35,36 @@ from wok.xmlutils.utils import xpath_get_text MAX_REDIRECTION_ALLOWED = 5 +def get_template_by_name(name): + + conn = sqlite3.connect(config.get_object_store(), timeout=10) + cursor = conn.cursor() + + # if * is passed: select all fields + sql = "SELECT json from objects where type=='template' and id=='%s'" % name + if name == "*": + sql = "SELECT json from objects where type == 'template'" + + # execute and fetch results + cursor.execute(sql) + content = cursor.fetchall() + + # no record: return nothing + if len(content) == 0: + return {} + + # sqllite returns a tuple of strings + # iterate over it and return a list of dictonaries + if len(content[0]) == 1: + return eval(content[0][0]) + + result = [] + for dictonary in content[0]: + result.append(eval(dictonary)) + + return result + + def _uri_to_name(collection, uri): expr = '/plugins/kimchi/%s/(.*?)$' % collection m = re.match(expr, uri) -- 2.5.5

First, improve functions inside class. This will improve code reading, also, make more functions to split procedures like: validating disks, graphics xml and distro checking. Add separate function to identify the installation media. Signed-off-by: Ramon Medeiros <ramonn at linux.vnet.ibm.com> --- model/templates.py | 4 ++ vmtemplate.py | 142 +++++++++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 132 insertions(+), 14 deletions(-) diff --git a/model/templates.py b/model/templates.py index b49ac50..a03a7cf 100644 --- a/model/templates.py +++ b/model/templates.py @@ -196,6 +196,10 @@ class TemplateModel(object): params['memory'].update(new_mem) validate_memory(params['memory']) + # params are cdrom or disk: add it to source_media + if "cdrom" in params: + params["source_media"] = params.get("cdrom") + new_t.update(params) for net_name in params.get(u'networks', []): diff --git a/vmtemplate.py b/vmtemplate.py index 4bcf324..8572dfe 100644 --- a/vmtemplate.py +++ b/vmtemplate.py @@ -17,6 +17,7 @@ # License along with this library; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +import magic import os import platform import stat @@ -29,10 +30,13 @@ from lxml.builder import E from wok.exception import InvalidParameter, ImageFormatError, IsoFormatError from wok.exception import MissingParameter, OperationFailed +from wok.xmlutils.utils import xpath_get_text from wok.plugins.kimchi import imageinfo from wok.plugins.kimchi import osinfo from wok.plugins.kimchi.isoinfo import IsoImage +from wok.plugins.kimchi.model.libvirtconnection import LibvirtConnection from wok.plugins.kimchi.utils import check_url_path, pool_name_from_uri +from wok.plugins.kimchi.utils import get_template_by_name from wok.plugins.kimchi.xmlutils.cpu import get_cpu_xml from wok.plugins.kimchi.xmlutils.disk import get_disk_xml from wok.plugins.kimchi.xmlutils.graphics import get_graphics_xml @@ -40,6 +44,9 @@ from wok.plugins.kimchi.xmlutils.interface import get_iface_xml from wok.plugins.kimchi.xmlutils.qemucmdline import get_qemucmdline_xml from wok.plugins.kimchi.xmlutils.serial import get_serial_xml +DISK_TYPE = {"QEMU QCOW Image": "qcow2", + "data": "raw"} +ISO_TYPE = "ISO 9660 CD-ROM" class VMTemplate(object): def __init__(self, args, scan=False): @@ -54,20 +61,16 @@ class VMTemplate(object): self.info = {} self.fc_host_support = args.get('fc_host_support') - # Fetch defaults based on the os distro and version - try: - distro, version = self._get_os_info(args, scan) - except ImageFormatError as e: - raise OperationFailed('KCHTMPL0020E', {'err': e.message}) - os_distro = args.get('os_distro', distro) - os_version = args.get('os_version', version) - entry = osinfo.lookup(os_distro, os_version) - self.info.update(entry) + # search for template with the same name + template = get_template_by_name(args.get("name")) - # Auto-generate a template name if no one is passed - if 'name' not in args or args['name'] == '': - args['name'] = self._gen_name(distro, version) - self.name = args['name'] + # no record found: create template + if len(template) == 0: + self._create_template(args, scan) + + # template already exists: load it + else: + self.info.update(args) # Merge graphics settings graph_args = args.get('graphics') @@ -87,6 +90,35 @@ class VMTemplate(object): # Override template values according to 'args' self.info.update(args) + + # validate disks + self._validate_disks(default_disk) + + def _create_template(self, args, scan=False): + """ + Creates a new template + """ + # identify installation media + self._identify_installation_media(args) + + # Fetch defaults based on the os distro and version + try: + distro, version = self._get_os_info(args, scan) + except ImageFormatError as e: + raise OperationFailed('KCHTMPL0020E', {'err': e.message}) + os_distro = args.get('os_distro', distro) + os_version = args.get('os_version', version) + entry = osinfo.lookup(os_distro, os_version) + + # update info + self.info.update(entry) + + # Auto-generate a template name if no one is passed + if 'name' not in args or args['name'] == '': + args['name'] = self._gen_name(distro, version) + self.info["name"] = args['name'] + + def _validate_disks(self, default_disk): disks = self.info.get('disks') basic_disk = ['index', 'format', 'pool', 'size'] @@ -95,7 +127,6 @@ class VMTemplate(object): for index, disk in enumerate(disks): disk_info = dict(default_disk) - pool_type = self._get_storage_type(disk['pool']['name']) if pool_type in ['iscsi', 'scsi']: disk_info = {'index': 0, 'format': 'raw', 'volume': None} @@ -118,6 +149,89 @@ class VMTemplate(object): disk_info['index'] = disk_info.get('index', index) self.info['disks'][index] = disk_info + def _identify_installation_media(self, args): + path = args.get("installation_media") + + # user did not passed installation media: return + if path is None: + return + + # create magic object to discover file type + file_type = magic.open(magic.MAGIC_NONE) + file_type.load() + + # file is local and exists: check if it's disk or cdrom + if path.startswith("/") and os.path.exists(path): + ftype = file_type.file(path) + + # cdrom + if ISO_TYPE in ftype: + args["cdrom"] = path + return + + # disk + for types in DISK_TYPE.keys(): + if types in ftype: + + # no disk list: create it + if args.get("disks") is None: + args["disks"] = [] + + # append disk to the list + args["disks"].insert(0, + {"base": path, + "format": DISK_TYPE[types], + "index": 0, + "pool": {"name": + "/plugins/kimchi/storagepools/default", + "type":"dir"}}) + + # get storage pool + pool = self._get_storagepool_from_storagevolume(path) + + # image not in any pool: get default pool + if len(pool) == 0: + os_info = osinfo.lookup("unknow", "unknow")["disks"][0]["pool"] + + # os_info has default pool: get it + if os_info != None: + args["disks"][0]["pool"] = os_info + return + + # pool available + args["disks"][0]["pool"] = pool + + # not local image: set as remote ISO + elif path.startswith("http") or path.startswith("ftp"): + args["cdrom"] = path + return + + def _get_storagepool_from_storagevolume(self, storagevolume_path): + conn = LibvirtConnection('qemu:///system').get() + storagepools = conn.listStoragePools() + storagepools += conn.listDefinedStoragePools() + + # discover storagepool + for storagepool in storagepools: + pool = conn.storagePoolLookupByName(storagepool) + spath = xpath_get_text(pool.XMLDesc(), "/pool/target/path")[0] + pool_type = xpath_get_text(pool.XMLDesc(), "/pool/@type")[0] + storagevolumes = sorted(map(lambda x: x.decode('utf-8'), + pool.listVolumes())) + for storagevolume in storagevolumes: + + # path does not exists: continue to the next + if not os.path.exists("%s/%s" % (spath, storagevolume)): + continue + + # storagepool found: return + volume = "%s/%s" % (spath, storagevolume) + if os.path.samefile(storagevolume_path, volume): + pool_uri = "/plugins/kimchi/storagepools/%s" % storagepool + return {"name": pool_uri, + "type": pool_type} + return {} + def _get_os_info(self, args, scan): distro = version = 'unknown' -- 2.5.5

Update all tests to use source_media instead of disks and cdrom --- tests/test_authorization.py | 4 +-- tests/test_livemigration.py | 7 +++-- tests/test_mockmodel.py | 14 ++++----- tests/test_model.py | 69 +++++++++++++++++++++++++++------------------ tests/test_rest.py | 32 +++++++++------------ tests/test_template.py | 39 ++++++++++--------------- tests/test_vmtemplate.py | 24 ++++++++-------- 7 files changed, 96 insertions(+), 93 deletions(-) diff --git a/tests/test_authorization.py b/tests/test_authorization.py index d88f763..7cf8da2 100644 --- a/tests/test_authorization.py +++ b/tests/test_authorization.py @@ -91,7 +91,7 @@ class AuthorizationTests(unittest.TestCase): # but he can get and create a new one resp = self.request('/plugins/kimchi/templates', '{}', 'GET') self.assertEquals(403, resp.status) - req = json.dumps({'name': 'test', 'cdrom': fake_iso}) + req = json.dumps({'name': 'test', 'source_media': fake_iso}) resp = self.request('/plugins/kimchi/templates', req, 'POST') self.assertEquals(403, resp.status) resp = self.request('/plugins/kimchi/templates/test', '{}', 'PUT') @@ -100,7 +100,7 @@ class AuthorizationTests(unittest.TestCase): self.assertEquals(403, resp.status) # Non-root users can only get vms authorized to them - model.templates_create({'name': u'test', 'cdrom': fake_iso}) + model.templates_create({'name': u'test', 'source_media': fake_iso}) task_info = model.vms_create({ 'name': u'test-me', diff --git a/tests/test_livemigration.py b/tests/test_livemigration.py index aa8d1bd..69407dc 100644 --- a/tests/test_livemigration.py +++ b/tests/test_livemigration.py @@ -97,12 +97,13 @@ class LiveMigrationTests(unittest.TestCase): ) params = {'name': u'template_test_vm_migrate', 'disks': [], - 'cdrom': UBUNTU_ISO, - 'memory': {'current': 2048, 'maxmemory': 4096 << 10}} + 'source_media': UBUNTU_ISO, + 'memory': 2048, + 'max_memory': 4096 << 10} self.inst.templates_create(params) params = {'name': u'template_test_vm_migrate_nonshared', 'disks': [{'name': 'test_vm_migrate.img', 'size': 1}], - 'cdrom': UBUNTU_ISO, + 'source_media': UBUNTU_ISO, 'memory': {'current': 2048, 'maxmemory': 4096*1024}} self.inst.templates_create(params) diff --git a/tests/test_mockmodel.py b/tests/test_mockmodel.py index 0668191..9a0ba74 100644 --- a/tests/test_mockmodel.py +++ b/tests/test_mockmodel.py @@ -30,6 +30,7 @@ from wok.exception import InvalidOperation from wok.plugins.kimchi import mockmodel from wok.plugins.kimchi.osinfo import get_template_default +import iso_gen test_server = None model = None @@ -50,8 +51,7 @@ def setUpModule(): test_server = run_server(host, port, ssl_port, test_mode=True, model=model) fake_iso = '/tmp/fake.iso' - open(fake_iso, 'w').close() - + iso_gen.construct_fake_iso(fake_iso, True, '12.04', 'ubuntu') def tearDown(): test_server.stop() @@ -65,7 +65,7 @@ class MockModelTests(unittest.TestCase): def test_screenshot_refresh(self): # Create a VM - req = json.dumps({'name': 'test', 'cdrom': fake_iso}) + req = json.dumps({'name': 'test', 'source_media': fake_iso}) request(host, ssl_port, '/plugins/kimchi/templates', req, 'POST') req = json.dumps({'name': 'test-vm', 'template': '/plugins/kimchi/templates/test'}) @@ -95,7 +95,7 @@ class MockModelTests(unittest.TestCase): resp.getheader('last-modified')) def test_vm_list_sorted(self): - req = json.dumps({'name': 'test', 'cdrom': fake_iso}) + req = json.dumps({'name': 'test', 'source_media': fake_iso}) request(host, ssl_port, '/plugins/kimchi/templates', req, 'POST') def add_vm(name): @@ -115,7 +115,7 @@ class MockModelTests(unittest.TestCase): def test_memory_window_changes(self): model.templates_create({'name': u'test', - 'cdrom': fake_iso}) + 'source_media': fake_iso}) task = model.vms_create({'name': u'test-vm', 'template': '/plugins/kimchi/templates/test'}) wait_task(model.task_lookup, task['id']) @@ -127,7 +127,7 @@ class MockModelTests(unittest.TestCase): def test_hotplug_3D_card(self): model.templates_create({'name': u'test', - 'cdrom': fake_iso}) + 'source_media': fake_iso}) task = model.vms_create({'name': u'test-vm', 'template': '/plugins/kimchi/templates/test'}) wait_task(model.task_lookup, task['id']) @@ -147,7 +147,7 @@ class MockModelTests(unittest.TestCase): def test_vm_info(self): model.templates_create({'name': u'test', - 'cdrom': fake_iso}) + 'source_media': fake_iso}) task = model.vms_create({'name': u'test-vm', 'template': '/plugins/kimchi/templates/test'}) wait_task(model.task_lookup, task['id']) diff --git a/tests/test_model.py b/tests/test_model.py index b461172..761da3c 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -140,7 +140,8 @@ class ModelTests(unittest.TestCase): params = {'name': 'test', 'disks': [{'base': vol['path'], 'size': 1, 'pool': { 'name': '/plugins/kimchi/storagepools/default'}}], - 'cdrom': UBUNTU_ISO} + 'source_media': UBUNTU_ISO} + inst.templates_create(params) rollback.prependDefer(inst.template_delete, 'test') @@ -162,15 +163,21 @@ class ModelTests(unittest.TestCase): self.assertRaises(InvalidOperation, inst.vmsnapshots_create, u'kimchi-vm') - - inst.vm_poweroff(u'kimchi-vm') - vm = inst.vm_lookup(u'kimchi-vm') + inst.vm_poweroff("kimchi-vm") + vm = inst.vm_lookup("kimchi-vm") empty_snap = inst.currentvmsnapshot_lookup(u'kimchi-vm') self.assertEquals({}, empty_snap) # this snapshot should be deleted when its VM is deleted params = {'name': u'mysnap'} + + # turnoff vm before snapshot + if inst.vm_lookup(u'kimchi-vm')['state'] == "running": + inst.vm_poweroff('kimchi-vm') + + vm = inst.vm_lookup(u'kimchi-vm') + task = inst.vmsnapshots_create(u'kimchi-vm', params) inst.task_wait(task['id']) task = inst.task_lookup(task['id']) @@ -179,6 +186,7 @@ class ModelTests(unittest.TestCase): self.assertRaises(NotFoundError, inst.vmsnapshot_lookup, u'kimchi-vm', u'foobar') + snap = inst.vmsnapshot_lookup(u'kimchi-vm', params['name']) self.assertTrue(int(time.time()) >= int(snap['created'])) self.assertEquals(vm['state'], snap['state']) @@ -207,6 +215,10 @@ class ModelTests(unittest.TestCase): current_snap = inst.currentvmsnapshot_lookup(u'kimchi-vm') self.assertEquals(snap, current_snap) + # "kimchi-vm-new" exists: remove it + if "kimchi-vm-new" in inst.vms_get_list(): + inst.vm_delete("kimchi-vm-new") + # update vm name inst.vm_update('kimchi-vm', {'name': u'kimchi-vm-new'}) @@ -216,7 +228,6 @@ class ModelTests(unittest.TestCase): # snapshot revert to the first created vm result = inst.vmsnapshot_revert(u'kimchi-vm-new', params['name']) self.assertEquals(result, [u'kimchi-vm', snap['name']]) - vm = inst.vm_lookup(u'kimchi-vm') self.assertEquals(vm['state'], snap['state']) @@ -259,22 +270,25 @@ class ModelTests(unittest.TestCase): 'capacity': 1073741824, # 1 GiB 'allocation': 1048576, # 1 MiB 'format': 'qcow2'} - task_id = inst.storagevolumes_create('default', params)['id'] + rollback.prependDefer(inst.storagevolume_delete, 'default', vol) - inst.task_wait(task_id) - self.assertEquals('finished', inst.task_lookup(task_id)['status']) + if "base-vol.img" not in inst.storagevolumes_get_list("default"): + task_id = inst.storagevolumes_create('default', params)['id'] + inst.task_wait(task_id) + self.assertEquals('finished', inst.task_lookup(task_id)['status']) vol_path = inst.storagevolume_lookup('default', vol)['path'] # Create template based on IMG file - tmpl_name = "img-tmpl" pool_uri = "/plugins/kimchi/storagepools/default" + tmpl_name = "img-tmpl" tmpl_info = {"cpu_info": {"vcpus": 1}, "name": tmpl_name, "graphics": {"type": "vnc", "listen": "127.0.0.1"}, "networks": ["default"], "memory": {'current': 1024}, "folder": [], "icon": "images/icon-vm.png", "cdrom": "", "os_distro": "unknown", "os_version": "unknown", - "disks": [{"base": vol_path, "size": 10, + "source_media": vol_path, + "disks": [{"size": 10, "format": "qcow2", "pool": {"name": pool_uri}}]} @@ -299,7 +313,7 @@ class ModelTests(unittest.TestCase): @unittest.skipUnless(utils.running_as_root(), 'Must be run as root') def test_vm_graphics(self): inst = model.Model(objstore_loc=self.tmp_store) - params = {'name': 'test', 'disks': [], 'cdrom': UBUNTU_ISO} + params = {'name': 'test', 'disks': [], 'source_media': UBUNTU_ISO} inst.templates_create(params) with RollbackContext() as rollback: params = {'name': 'kimchi-vnc', @@ -329,7 +343,7 @@ class ModelTests(unittest.TestCase): @unittest.skipUnless(utils.running_as_root(), "Must be run as root") def test_vm_serial(self): inst = model.Model(objstore_loc=self.tmp_store) - params = {'name': 'test', 'disks': [], 'cdrom': UBUNTU_ISO} + params = {'name': 'test', 'disks': [], 'source_media': UBUNTU_ISO} inst.templates_create(params) with RollbackContext() as rollback: params = {'name': 'kimchi-serial', @@ -350,7 +364,7 @@ class ModelTests(unittest.TestCase): def test_vm_ifaces(self): inst = model.Model(objstore_loc=self.tmp_store) with RollbackContext() as rollback: - params = {'name': 'test', 'disks': [], 'cdrom': UBUNTU_ISO} + params = {'name': 'test', 'disks': [], 'source_media': UBUNTU_ISO} inst.templates_create(params) rollback.prependDefer(inst.template_delete, 'test') @@ -466,7 +480,7 @@ class ModelTests(unittest.TestCase): inst.task_wait(task_id) vm_name = 'kimchi-cdrom' - params = {'name': 'test', 'disks': [], 'cdrom': UBUNTU_ISO} + params = {'name': 'test','disks':[], 'source_media': UBUNTU_ISO} inst.templates_create(params) rollback.prependDefer(inst.template_delete, 'test') params = {'name': vm_name, @@ -510,7 +524,7 @@ class ModelTests(unittest.TestCase): vm_name = 'kimchi-ide-bus-vm' params = {'name': 'old_distro_template', 'disks': [], - 'cdrom': old_distro_iso} + 'source_media': old_distro_iso} inst.templates_create(params) rollback.prependDefer(inst.template_delete, 'old_distro_template') params = { @@ -535,7 +549,7 @@ class ModelTests(unittest.TestCase): inst = model.Model(objstore_loc=self.tmp_store) with RollbackContext() as rollback: vm_name = 'kimchi-cdrom' - params = {'name': 'test', 'disks': [], 'cdrom': UBUNTU_ISO} + params = {'name': 'test',"disks":[], 'source_media': UBUNTU_ISO} inst.templates_create(params) rollback.prependDefer(inst.template_delete, 'test') params = {'name': vm_name, @@ -626,7 +640,8 @@ class ModelTests(unittest.TestCase): with RollbackContext() as rollback: params = {'name': 'test', 'disks': [{'size': 1, 'pool': { 'name': '/plugins/kimchi/storagepools/default'}}], - 'cdrom': UBUNTU_ISO} + 'source_media': UBUNTU_ISO} + inst.templates_create(params) rollback.prependDefer(inst.template_delete, 'test') @@ -685,7 +700,7 @@ class ModelTests(unittest.TestCase): params = {'name': 'test', 'disks': [{'size': 1, 'pool': { 'name': '/plugins/kimchi/storagepools/default'}}], - 'cdrom': UBUNTU_ISO} + 'source_media': UBUNTU_ISO} inst.templates_create(params) rollback.prependDefer(inst.template_delete, 'test') @@ -713,7 +728,8 @@ class ModelTests(unittest.TestCase): params = {'name': 'test', 'disks': [{ 'size': 1, 'format': user_vol, 'pool': {'name': '/plugins/kimchi/storagepools/default'}}], - 'cdrom': UBUNTU_ISO} + 'source_media': UBUNTU_ISO} + inst.templates_create(params) rollback.prependDefer(inst.template_delete, 'test') @@ -738,7 +754,7 @@ class ModelTests(unittest.TestCase): params = {'name': 'test', 'disks': [{'size': 1, 'pool': { 'name': '/plugins/kimchi/storagepools/default'}}], - 'cdrom': UBUNTU_ISO} + 'source_media': UBUNTU_ISO} inst.templates_create(params) rollback.prependDefer(inst.template_delete, 'test') @@ -758,7 +774,7 @@ class ModelTests(unittest.TestCase): inst = model.Model(None, objstore_loc=self.tmp_store) orig_params = {'name': 'test', 'memory': {'current': 1024, 'maxmemory': 3072}, - 'cdrom': UBUNTU_ISO} + 'source_media': UBUNTU_ISO} inst.templates_create(orig_params) with RollbackContext() as rollback: @@ -795,9 +811,9 @@ class ModelTests(unittest.TestCase): # only supports this format orig_params = {'name': 'test', 'memory': {'current': 1024}, 'cpu_info': {'vcpus': 1}, - 'cdrom': UBUNTU_ISO, 'disks': [{'size': 1, 'format': 'qcow2', 'pool': { - 'name': '/plugins/kimchi/storagepools/default'}}]} + 'name': '/plugins/kimchi/storagepools/default'}}], + 'source_media': UBUNTU_ISO} inst.templates_create(orig_params) with RollbackContext() as rollback: @@ -1033,7 +1049,7 @@ class ModelTests(unittest.TestCase): inst = model.Model(objstore_loc=self.tmp_store) with RollbackContext() as rollback: - params = {'name': u'test', 'disks': [], 'cdrom': UBUNTU_ISO} + params = {'name': u'test', 'source_media': UBUNTU_ISO} inst.templates_create(params) rollback.prependDefer(inst.template_delete, 'test') @@ -1059,7 +1075,7 @@ class ModelTests(unittest.TestCase): inst = model.Model(objstore_loc=self.tmp_store) with RollbackContext() as rollback: - params = {'name': 'test', 'disks': [], 'cdrom': UBUNTU_ISO} + params = {'name': 'test', 'source_media': UBUNTU_ISO} inst.templates_create(params) rollback.prependDefer(inst.template_delete, 'test') @@ -1130,8 +1146,7 @@ class ModelTests(unittest.TestCase): with RollbackContext() as rollback: params = { 'name': 'test', - 'disks': [], - 'cdrom': UBUNTU_ISO, + 'source_media': UBUNTU_ISO, 'domain': 'test', 'arch': 'i686' } diff --git a/tests/test_rest.py b/tests/test_rest.py index d0d2fcf..a76cbcb 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -107,7 +107,7 @@ class RestTests(unittest.TestCase): self.assertEquals(1, len(vms)) # Create a template as a base for our VMs - req = json.dumps({'name': 'test', 'cdrom': fake_iso}) + req = json.dumps({'name': 'test', 'source_media': fake_iso}) resp = self.request('/plugins/kimchi/templates', req, 'POST') self.assertEquals(201, resp.status) @@ -134,7 +134,7 @@ class RestTests(unittest.TestCase): self.assertEquals([], vm['groups']) def test_edit_vm(self): - req = json.dumps({'name': 'test', 'cdrom': fake_iso}) + req = json.dumps({'name': 'test', 'source_media': fake_iso}) resp = self.request('/plugins/kimchi/templates', req, 'POST') self.assertEquals(201, resp.status) @@ -291,9 +291,8 @@ class RestTests(unittest.TestCase): def test_vm_lifecycle(self): # Create a Template - req = json.dumps({'name': 'test', 'disks': DISKS, - 'icon': 'plugins/kimchi/images/icon-debian.png', - 'cdrom': fake_iso}) + req = json.dumps({'name': 'test', 'icon': 'plugins/kimchi/images/icon-debian.png', + 'source_media': fake_iso,'disks': DISKS}) resp = self.request('/plugins/kimchi/templates', req, 'POST') self.assertEquals(201, resp.status) @@ -518,7 +517,7 @@ class RestTests(unittest.TestCase): def test_vm_graphics(self): # Create a Template - req = json.dumps({'name': 'test', 'cdrom': fake_iso}) + req = json.dumps({'name': 'test', 'source_media': fake_iso}) resp = self.request('/plugins/kimchi/templates', req, 'POST') self.assertEquals(201, resp.status) @@ -612,7 +611,7 @@ class RestTests(unittest.TestCase): with RollbackContext() as rollback: # Create a template as a base for our VMs - req = json.dumps({'name': 'test', 'cdrom': fake_iso}) + req = json.dumps({'name': 'test', 'source_media': fake_iso}) resp = self.request('/plugins/kimchi/templates', req, 'POST') self.assertEquals(201, resp.status) # Delete the template @@ -785,7 +784,7 @@ class RestTests(unittest.TestCase): with RollbackContext() as rollback: # Create a template as a base for our VMs - req = json.dumps({'name': 'test', 'cdrom': fake_iso}) + req = json.dumps({'name': 'test', 'source_media': fake_iso}) resp = self.request('/plugins/kimchi/templates', req, 'POST') self.assertEquals(201, resp.status) # Delete the template @@ -891,8 +890,7 @@ class RestTests(unittest.TestCase): def test_vm_customise_storage(self): # Create a Template - req = json.dumps({'name': 'test', 'cdrom': fake_iso, - 'disks': DISKS}) + req = json.dumps({'name': 'test', 'source_media': fake_iso, 'disks': DISKS}) resp = self.request('/plugins/kimchi/templates', req, 'POST') self.assertEquals(201, resp.status) @@ -954,7 +952,7 @@ class RestTests(unittest.TestCase): # Create template fails because SCSI volume is missing tmpl_params = { - 'name': 'test_fc_pool', 'cdrom': fake_iso, 'disks': [{'pool': { + 'name': 'test_fc_pool', 'source_media': fake_iso, 'disks': [{'pool': { 'name': '/plugins/kimchi/storagepools/scsi_fc_pool'}}]} req = json.dumps(tmpl_params) @@ -998,7 +996,7 @@ class RestTests(unittest.TestCase): def test_unnamed_vms(self): # Create a Template - req = json.dumps({'name': 'test', 'cdrom': fake_iso}) + req = json.dumps({'name': 'test', 'source_media': fake_iso}) resp = self.request('/plugins/kimchi/templates', req, 'POST') self.assertEquals(201, resp.status) @@ -1039,10 +1037,8 @@ class RestTests(unittest.TestCase): # Create a Template mock_base = '/tmp/mock.img' - open(mock_base, 'w').close() - disks = copy.deepcopy(DISKS) - disks[0]['base'] = mock_base - req = json.dumps({'name': 'test', 'disks': disks}) + os.system("qemu-img create -f qcow2 %s 10M" % mock_base) + req = json.dumps({'name': 'test', 'source_media': mock_base}) resp = self.request('/plugins/kimchi/templates', req, 'POST') self.assertEquals(201, resp.status) @@ -1112,7 +1108,7 @@ class RestTests(unittest.TestCase): # In real model os distro/version can be omitted # as we will scan the iso req = json.dumps({'name': 'test', - 'cdrom': storagevolume['path'], + 'source_media': storagevolume['path'], 'os_distro': storagevolume['os_distro'], 'os_version': storagevolume['os_version']}) resp = self.request('/plugins/kimchi/templates', req, 'POST') @@ -1150,7 +1146,7 @@ class RestTests(unittest.TestCase): def test_screenshot_refresh(self): # Create a VM - req = json.dumps({'name': 'test', 'cdrom': fake_iso}) + req = json.dumps({'name': 'test', 'source_media': fake_iso}) resp = self.request('/plugins/kimchi/templates', req, 'POST') req = json.dumps({'name': 'test-vm', 'template': '/plugins/kimchi/templates/test'}) diff --git a/tests/test_template.py b/tests/test_template.py index fcb2e46..f7f6616 100644 --- a/tests/test_template.py +++ b/tests/test_template.py @@ -17,7 +17,7 @@ # You should have received a copy of the GNU Lesser General Public # License along with this library; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA - +import iso_gen import json import os import psutil @@ -53,6 +53,7 @@ def setUpModule(): cherrypy_port = get_free_port('cherrypy_port') test_server = run_server(host, port, ssl_port, test_mode=True, cherrypy_port=cherrypy_port, model=model) + iso_gen.construct_fake_iso("/tmp/mock.iso", True, '14.04', 'ubuntu') def tearDownModule(): @@ -70,16 +71,16 @@ class TemplateTests(unittest.TestCase): self.assertEquals(200, resp.status) self.assertEquals(0, len(json.loads(resp.read()))) - # Create a template without cdrom and disk specified fails with 400 + # Create a template without cdrom and disk specified fails with 500 t = {'name': 'test', 'os_distro': 'ImagineOS', 'os_version': '1.0', 'memory': {'current': 1024}, 'cpu_info': {'vcpus': 1}} req = json.dumps(t) resp = self.request('/plugins/kimchi/templates', req, 'POST') - self.assertEquals(400, resp.status) + self.assertEquals(500, resp.status) # Create a template - t = {'name': 'test', 'cdrom': '/tmp/mock.iso'} + t = {'name': 'test', 'source_media': '/tmp/mock.iso'} req = json.dumps(t) resp = self.request('/plugins/kimchi/templates', req, 'POST') self.assertEquals(201, resp.status) @@ -117,22 +118,20 @@ class TemplateTests(unittest.TestCase): self.assertEquals(204, resp.status) # Create a template with same name fails with 400 - req = json.dumps({'name': 'test', 'cdrom': '/tmp/mock.iso'}) + req = json.dumps({'name': 'test', 'source_media': '/tmp/mock.iso'}) resp = self.request('/plugins/kimchi/templates', req, 'POST') self.assertEquals(400, resp.status) # Create an image based template - open('/tmp/mock.img', 'w').close() - t = {'name': 'test_img_template', 'disks': [{ - 'base': '/tmp/mock.img', 'format': 'qcow2', - 'pool': {'name': DEFAULT_POOL}, 'size': 1}]} + os.system("qemu-img create -f qcow2 %s 10G" % '/tmp/mock.img') + t = {'name': 'test_img_template', 'source_media': '/tmp/mock.img'} req = json.dumps(t) resp = self.request('/plugins/kimchi/templates', req, 'POST') self.assertEquals(201, resp.status) os.remove('/tmp/mock.img') # Test disk format - t = {'name': 'test-format', 'cdrom': '/tmp/mock.iso', 'disks': [{ + t = {'name': 'test-format', 'source_media': '/tmp/mock.iso', 'disks': [{ 'index': 0, 'size': 10, 'format': 'vmdk', 'pool': { 'name': DEFAULT_POOL}}]} req = json.dumps(t) @@ -149,7 +148,7 @@ class TemplateTests(unittest.TestCase): else: max_mem = (psutil.TOTAL_PHYMEM >> 10 >> 10) memory = max_mem + 1024 - t = {'name': 'test-maxmem', 'cdrom': '/tmp/mock.iso', + t = {'name': 'test-maxmem', 'source_media': '/tmp/mock.iso', 'memory': {'current': memory}} req = json.dumps(t) resp = self.request('/plugins/kimchi/templates', req, 'POST') @@ -158,7 +157,7 @@ class TemplateTests(unittest.TestCase): def test_customized_tmpl(self): # Create a template - t = {'name': 'test', 'cdrom': '/tmp/mock.iso'} + t = {'name': 'test', 'source_media': '/tmp/mock.iso'} req = json.dumps(t) resp = self.request('/plugins/kimchi/templates', req, 'POST') self.assertEquals(201, resp.status) @@ -328,7 +327,7 @@ class TemplateTests(unittest.TestCase): def test_customized_network(self): # Create a template - t = {'name': 'test', 'cdrom': '/tmp/mock.iso'} + t = {'name': 'test', 'source_media': '/tmp/mock.iso'} req = json.dumps(t) resp = self.request('/plugins/kimchi/templates', req, 'POST') self.assertEquals(201, resp.status) @@ -364,7 +363,7 @@ class TemplateTests(unittest.TestCase): def test_customized_storagepool(self): # Create a template - t = {'name': 'test', 'cdrom': '/tmp/mock.iso'} + t = {'name': 'test', 'source_media': '/tmp/mock.iso'} req = json.dumps(t) resp = self.request('/plugins/kimchi/templates', req, 'POST') self.assertEquals(201, resp.status) @@ -453,12 +452,8 @@ class TemplateTests(unittest.TestCase): self.request(pool_uri + '/activate', '{}', 'POST') # Create a template using the custom network and pool - t = {'name': 'test', 'cdrom': '/tmp/mock.iso', - 'networks': ['nat-network'], - 'disks': [{'pool': { - 'name': '/plugins/kimchi/storagepools/dir-pool'}, - 'size': 2, - 'format': 'qcow2'}]} + t = {'name': 'test', 'source_media': '/tmp/mock.iso', + 'networks': ['nat-network']} req = json.dumps(t) resp = self.request('/plugins/kimchi/templates', req, 'POST') self.assertEquals(201, resp.status) @@ -482,7 +477,3 @@ class TemplateTests(unittest.TestCase): resp = self.request('/plugins/kimchi/storagepools/dir-pool', '{}', 'DELETE') self.assertEquals(400, resp.status) - - # Verify the template - res = json.loads(self.request('/plugins/kimchi/templates/test').read()) - self.assertEquals(res['invalid']['cdrom'], ['/tmp/mock.iso']) diff --git a/tests/test_vmtemplate.py b/tests/test_vmtemplate.py index eed58b0..844a827 100644 --- a/tests/test_vmtemplate.py +++ b/tests/test_vmtemplate.py @@ -17,6 +17,7 @@ # License along with this library; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +import iso_gen import os import psutil import unittest @@ -36,8 +37,7 @@ DISKS = [{'size': 10, 'format': 'raw', 'index': 0, 'pool': {'name': class VMTemplateTests(unittest.TestCase): def setUp(self): self.iso = '/tmp/mock.iso' - open(self.iso, 'w').close() - + iso_gen.construct_fake_iso(self.iso, True, '12.04', 'ubuntu') def tearDown(self): os.unlink(self.iso) @@ -45,22 +45,22 @@ class VMTemplateTests(unittest.TestCase): disk_bus = get_template_default('old', 'disk_bus') memory = get_template_default('old', 'memory') nic_model = get_template_default('old', 'nic_model') - fields = (('name', 'test'), ('cdrom', self.iso), + fields = (('name', 'test'), ('source_media', self.iso), ('os_distro', 'unknown'), ('os_version', 'unknown'), ('cpu_info', {'vcpus': 1, 'maxvcpus': 1}), ('memory', memory), ('networks', ['default']), ('disk_bus', disk_bus), ('nic_model', nic_model), ('graphics', {'type': 'vnc', 'listen': '127.0.0.1'})) - args = {'name': 'test', 'cdrom': self.iso} + args = {'name': 'test', 'source_media': self.iso} t = VMTemplate(args) for name, val in fields: self.assertEquals(val, t.info.get(name)) def test_construct_overrides(self): graphics = {'type': 'spice', 'listen': '127.0.0.1'} - args = {'name': 'test', 'disks': DISKS, - 'graphics': graphics, "cdrom": self.iso} + args = {'name': 'test', 'graphics': graphics, 'disks': DISKS, + 'source_media': self.iso} t = VMTemplate(args) self.assertEquals(2, len(t.info['disks'])) self.assertEquals(graphics, t.info['graphics']) @@ -68,8 +68,8 @@ class VMTemplateTests(unittest.TestCase): def test_specified_graphics(self): # Test specified listen graphics = {'type': 'vnc', 'listen': '127.0.0.1'} - args = {'name': 'test', 'disks': DISKS, - 'graphics': graphics, 'cdrom': self.iso} + args = {'name': 'test', 'graphics': graphics, 'disks': DISKS, + 'source_media': self.iso} t = VMTemplate(args) self.assertEquals(graphics, t.info['graphics']) @@ -89,7 +89,7 @@ class VMTemplateTests(unittest.TestCase): def test_to_xml(self): graphics = {'type': 'spice', 'listen': '127.0.0.1'} vm_uuid = str(uuid.uuid4()).replace('-', '') - t = VMTemplate({'name': 'test-template', 'cdrom': self.iso}) + t = VMTemplate({'name': 'test-template', 'source_media': self.iso}) xml = t.to_vm_xml('test-vm', vm_uuid, graphics=graphics) self.assertEquals(vm_uuid, xpath_get_text(xml, "/domain/uuid")[0]) self.assertEquals('test-vm', xpath_get_text(xml, "/domain/name")[0]) @@ -108,7 +108,7 @@ class VMTemplateTests(unittest.TestCase): host_memory = psutil.virtual_memory().total >> 10 else: host_memory = psutil.TOTAL_PHYMEM >> 10 - t = VMTemplate({'name': 'test-template', 'cdrom': self.iso, + t = VMTemplate({'name': 'test-template', 'source_media': self.iso, 'memory': {'current': (host_memory >> 10) - 512}}) try: xml = t.to_vm_xml('test-vm', vm_uuid, graphics=graphics) @@ -125,12 +125,12 @@ class VMTemplateTests(unittest.TestCase): args = {'name': 'test', 'os_distro': 'opensuse', 'os_version': '12.3', 'cpu_info': {'vcpus': 2, 'maxvcpus': 4}, 'memory': {'current': 2048, 'maxmemory': 3072}, - 'networks': ['foo'], 'cdrom': self.iso, 'graphics': graphics} + 'networks': ['foo'], 'source_media': self.iso, 'graphics': graphics} t = VMTemplate(args) self.assertEquals(2, t.info.get('cpu_info', {}).get('vcpus')) self.assertEquals(4, t.info.get('cpu_info', {}).get('maxvcpus')) self.assertEquals(2048, t.info.get('memory').get('current')) self.assertEquals(3072, t.info.get('memory').get('maxmemory')) self.assertEquals(['foo'], t.info.get('networks')) - self.assertEquals(self.iso, t.info.get('cdrom')) + self.assertEquals(self.iso, t.info.get('source_media')) self.assertEquals(graphics, t.info.get('graphics')) -- 2.5.5

Nice catch to check if the template already exists a little bit early than current code. But I guess would be better if you do the check in the beginning of the method, preventing the execution of all unnecessary process the method does and will not be used. What do you think? On 03/22/2016 04:20 PM, Ramon Medeiros wrote:

On 03/23/2016 10:44 AM, Paulo Ricardo Paz Vital wrote: lookup uses the session object, which is not available at templates. we can change the * -- Ramon Nunes Medeiros Kimchi Developer Linux Technology Center Brazil IBM Systems & Technology Group Phone : +55 19 2132 7878 ramonn(a)br.ibm.com

Is still possible to create a template by specifying 'cdrom' or base disk? If not, the API.json should be updated to block it and required installation_media always. On 03/22/2016 04:20 PM, Ramon Medeiros wrote:

On 03/22/2016 04:20 PM, Ramon Medeiros wrote: Is it 'source_media' ou 'installation_media'? I prefer 'source_media' but if so, you need to update API.json (first patch) to reflect it. Also, if the user is already using the 'cdrom' or base disk as parameter, why do you need to check the media type? VMTemplate must be independent of Model. Instead of doing that, you should pass the data you want as parameter to the function. You are mixing the Template resource provided by model/templates.py to VMTemplate. VMTemplate is a dict of information. At this point, there is no information stored anywhere. I think you are doing it to do no scan the source_media multiple times, but for that the 'scan' parameter was used. You should continue to use it. Why a new function is needed? Can't we only use the existing functions to identify an ISO or image? You are mixing 'installation_media' with 'source_media' There is a function on utils.py for that - check_url_path() All that should not be on VMTemplate. As I said before VMTemplate is independent of Model. Any information you want should go through Model via parameter.

On 03/23/2016 05:50 PM, Ramon Medeiros wrote: The real problem here is that you are mixing the model with vmtemplate The model is responsible to store and deal with vmtemplate data. VMTemplate only gets the data, do some merges with default values and provide functions to export the guest XML.