On 03/31/2016 10:47 AM, Ramon Medeiros wrote:

Signed-off-by: Ramon Medeiros <ramonn@linux.vnet.ibm.com> --- API.json | 5 +++++ i18n.py | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/API.json b/API.json index 6f0b484..ce25fe1 100644 --- a/API.json +++ b/API.json @@ -468,6 +468,11 @@ "error": "KCHTMPL0011E" }, "memory": { "$ref": "#/kimchitype/memory" }, + "source_media": { + "description": "Path for installation media (ISO, disk, remote ISO)", + "type" : "string", + "pattern" : "^[^ ]+( +[^ ]+)*$" + }, "cdrom": { "description": "Path for cdrom", "type": "string",

You should remove cdrom and disk[base] from API.json and make source_media an required parameter to create a template.

diff --git a/i18n.py b/i18n.py index 6214687..57b033d 100644 --- a/i18n.py +++ b/i18n.py @@ -168,7 +168,7 @@ messages = { "KCHTMPL0013E": _("Amount of memory and maximum memory (MB) must be an integer greater than 512"), "KCHTMPL0014E": _("Template CDROM must be a local or remote ISO file"), "KCHTMPL0015E": _("Invalid storage pool URI %(value)s specified for template"), - "KCHTMPL0016E": _("Specify an ISO image as CDROM or a base image to create a template"), + "KCHTMPL0016E": _("Specify a path to source media (ISO, disk or remote ISO) to create a template"), "KCHTMPL0017E": _("All networks for the template must be specified in a list."), "KCHTMPL0018E": _("Specify a volume to a template when storage pool is iSCSI or SCSI"), "KCHTMPL0019E": _("The volume %(volume)s is not in storage pool %(pool)s"),

On 03/31/2016 10:47 AM, Ramon Medeiros wrote:

Template was being check if duplicated after calling for VMTemplate, which can make unecessary work. Check it before creating the class.

Signed-off-by: Ramon Medeiros <ramonn@linux.vnet.ibm.com> --- model/templates.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/model/templates.py b/model/templates.py index 92705b6..28c8a08 100644 --- a/model/templates.py +++ b/model/templates.py @@ -52,6 +52,12 @@ class TemplatesModel(object): def create(self, params): name = params.get('name', '').strip()

iso = params.get('cdrom')

cdrom should be removed. The single way to create a template is by specifying a source_media value.

+ + # template with the same name already exists: raise exception + with self.objstore as session: + if name in session.get_list('template'): + raise InvalidOperation("KCHTMPL0001E", {'name': name}) +

The template name is optional. So once user has send an empty name or no value for it, Kimchi should determine one name automatically. Following this patch, in this scenario, you are allowing user creates a template name with an empty string.

# check search permission if iso and iso.startswith('/') and os.path.exists(iso): st_mode = os.stat(iso).st_mode @@ -72,8 +78,7 @@ class TemplatesModel(object): raise InvalidParameter("KCHTMPL0003E", {'network': net_name, 'template': name}) # Creates the template class with necessary information - # Checkings will be done while creating this class, so any exception - # will be raised here + t = LibvirtVMTemplate(params, scan=True, conn=self.conn)

# Validate cpu info @@ -93,8 +98,6 @@ class TemplatesModel(object): name = params['name'] try: with self.objstore as session: - if name in session.get_list('template'): - raise InvalidOperation("KCHTMPL0001E", {'name': name}) session.store('template', name, t.info, get_kimchi_version()) except InvalidOperation:

On 03/31/2016 10:47 AM, Ramon Medeiros wrote:

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@linux.vnet.ibm.com> --- model/templates.py | 4 ++ vmtemplate.py | 110 +++++++++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 98 insertions(+), 16 deletions(-)

diff --git a/model/templates.py b/model/templates.py index 28c8a08..30d403e 100644 --- a/model/templates.py +++ b/model/templates.py @@ -198,6 +198,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") +

I understood from previous patch the above code is for template update but in any case you already know it is a cdrom, why you are using source_media to determine the type of the source media? Seems an overhead process.

new_t.update(params)

for net_name in params.get(u'networks', []): diff --git a/vmtemplate.py b/vmtemplate.py index 653ad02..958725a 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 @@ -40,6 +41,10 @@ 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 +59,13 @@ 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) + # no record found: create template + if scan: + self._create_template(args, scan)

- # 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'] + # template already exists: load it + else: + self.info.update(args)

# Merge graphics settings graph_args = args.get('graphics') @@ -76,8 +74,6 @@ class VMTemplate(object): graphics.update(graph_args) args['graphics'] = graphics

- default_disk = self.info['disks'][0] - # Complete memory args, because dict method update is not recursive if 'memory' in args: if 'current' not in args['memory']: @@ -87,6 +83,42 @@ class VMTemplate(object):

# Override template values according to 'args' self.info.update(args) + + if self.info.get("disks") is not None and len(self.info["disks"]) >= 1: + default_disk = self.info['disks'][0] + + # validate disks + self._validate_disks(default_disk) + + def _create_template(self, args, scan=False): + """ + Creates a new template + """

+ # no source_media argument: raise error + if args.get("source_media") is None: + raise MissingParameter('KCHTMPL0016E') +

This should be contained on API.json as marking source_media as required parameter. And FYI, any logic needs to be on model instead of vmtemplate

+ # identify source 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.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,53 @@ 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("source_media") +

+ # user did not passed source media: return + if path is None: + return +

Path should not be None at this part.

+ # 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

+ default_disk = { + "base": path, "format": DISK_TYPE[types], "index": 0, + "pool": { + "name": "/plugins/kimchi/storagepools/default", + "type": "dir"}}

You are assuming values here that should come from osinfo details.

+ args["disks"].insert(0, default_disk) + osi = osinfo.lookup("unknow", "unknow")["disks"][0]["pool"] +

Why you are doing it? Assuming unknow to lookup data?

+ # os_info has default pool: get it + if osi is not None: + args["disks"][0]["pool"] = osi + return + + # not local image: set as remote ISO + elif check_url_path(path): + args["cdrom"] = path + return + def _get_os_info(self, args, scan): distro = version = 'unknown'

On 03/31/2016 10:47 AM, Ramon Medeiros wrote:

Update all tests to use source_media instead of disks and cdrom

Signed-off-by: Ramon Medeiros <ramonn@linux.vnet.ibm.com> --- tests/test_authorization.py | 4 +- tests/test_livemigration.py | 7 ++-- tests/test_mockmodel.py | 13 ++++--- tests/test_model.py | 94 ++++++++++++++++++++++++--------------------- tests/test_rest.py | 36 ++++++++--------- tests/test_template.py | 47 ++++++++++------------- tests/test_vmtemplate.py | 40 ++++++++++--------- 7 files changed, 124 insertions(+), 117 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..34fe853 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': {'current': 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..41156f9 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,7 +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(): @@ -65,7 +66,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 +96,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 +116,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 +128,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 +148,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 c91ba8f..52fdced 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -138,9 +138,10 @@ class ModelTests(unittest.TestCase): vol = inst.storagevolume_lookup(u'default', vol_params['name'])

params = {'name': 'test', 'disks': - [{'base': vol['path'], 'size': 1, 'pool': { - 'name': '/plugins/kimchi/storagepools/default'}}], - 'cdrom': UBUNTU_ISO} + [{'index': 0, 'base': vol['path'], 'size': 1, 'pool': { + 'name': '/plugins/kimchi/storagepools/default'}, + 'format': 'qcow2'}], 'source_media': UBUNTU_ISO} + inst.templates_create(params) rollback.prependDefer(inst.template_delete, 'test')

@@ -162,7 +163,6 @@ 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')

@@ -171,6 +171,9 @@ class ModelTests(unittest.TestCase):

# this snapshot should be deleted when its VM is deleted params = {'name': u'mysnap'} + + 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']) @@ -208,7 +211,7 @@ class ModelTests(unittest.TestCase): self.assertEquals(snap, current_snap)

# update vm name - inst.vm_update('kimchi-vm', {'name': u'kimchi-vm-new'}) + inst.vm_update(u'kimchi-vm', {'name': u'kimchi-vm-new'})

# Look up the first created snapshot from the renamed vm snap = inst.vmsnapshot_lookup(u'kimchi-vm-new', params['name']) @@ -216,7 +219,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'])

@@ -247,7 +249,7 @@ class ModelTests(unittest.TestCase): inst.vm_suspend(u'kimchi-vm')

vms = inst.vms_get_list() - self.assertFalse('kimchi-vm' in vms) + self.assertFalse(u'kimchi-vm' in vms)

@unittest.skipUnless(utils.running_as_root(), 'Must be run as root') def test_image_based_template(self): @@ -259,10 +261,14 @@ 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"):

Why is it needed? base-vol.img is only used by the test and should be removed when test ends so it should not exist in the pool at this time.

+ 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']) vol_path = inst.storagevolume_lookup('default', vol)['path']

# Create template based on IMG file @@ -272,9 +278,9 @@ class ModelTests(unittest.TestCase): "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, + "os_distro": "unknown", "os_version": "unknown", + "source_media": vol_path, + "disks": [{"size": 10, "format": "qcow2", "pool": {"name": pool_uri}}]}

@@ -299,7 +305,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 +335,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 +356,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 +472,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 +516,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 +541,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, @@ -624,9 +630,10 @@ class ModelTests(unittest.TestCase): inst = model.Model(objstore_loc=self.tmp_store)

with RollbackContext() as rollback: - params = {'name': 'test', 'disks': [{'size': 1, 'pool': { - 'name': '/plugins/kimchi/storagepools/default'}}], - 'cdrom': UBUNTU_ISO} + params = {'name': 'test', 'disks': [{'size': 1, 'index': 0, + 'pool': {'name': '/plugins/kimchi/storagepools/default'},

+ 'format': 'qcow2'}], 'source_media': UBUNTU_ISO} +

Why have you added format parameter?

inst.templates_create(params) rollback.prependDefer(inst.template_delete, 'test')

@@ -683,9 +690,9 @@ class ModelTests(unittest.TestCase): self._create_template_conf_with_disk_format('vmdk') rollback.prependDefer(self._restore_template_conf_file)

- params = {'name': 'test', 'disks': [{'size': 1, 'pool': { - 'name': '/plugins/kimchi/storagepools/default'}}], - 'cdrom': UBUNTU_ISO} + params = {'name': 'test', 'disks': [{'index': 0, 'size': 1, + 'pool': {'name': '/plugins/kimchi/storagepools/default'}, + 'format': 'vmdk'}], 'source_media': UBUNTU_ISO}

Same here.

inst.templates_create(params) rollback.prependDefer(inst.template_delete, 'test')

@@ -710,10 +717,10 @@ class ModelTests(unittest.TestCase): self._create_template_conf_with_disk_format(default_vol) rollback.prependDefer(self._restore_template_conf_file)

- params = {'name': 'test', 'disks': [{ - 'size': 1, 'format': user_vol, - 'pool': {'name': '/plugins/kimchi/storagepools/default'}}], - 'cdrom': UBUNTU_ISO} + params = {'name': 'test', 'disks': [{'index': 0, 'size': 1, + 'pool': {'name': '/plugins/kimchi/storagepools/default'}, + 'format': 'raw'}], 'source_media': UBUNTU_ISO} +

And here.

inst.templates_create(params) rollback.prependDefer(inst.template_delete, 'test')

@@ -736,9 +743,9 @@ class ModelTests(unittest.TestCase): self._create_template_conf_with_disk_format(None) rollback.prependDefer(self._restore_template_conf_file)

- params = {'name': 'test', 'disks': [{'size': 1, 'pool': { - 'name': '/plugins/kimchi/storagepools/default'}}], - 'cdrom': UBUNTU_ISO} + params = {'name': 'test', 'disks': [{'index': 0, 'size': 1, + 'pool': {'name': '/plugins/kimchi/storagepools/default'}, + 'format': 'qcow2'}], 'source_media': UBUNTU_ISO}

And here.

inst.templates_create(params) rollback.prependDefer(inst.template_delete, 'test')

@@ -758,7 +765,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: @@ -793,11 +800,13 @@ class ModelTests(unittest.TestCase):

# template disk format must be qcow2 because vmsnapshot # 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'}}]} + orig_params = { + 'name': 'test', 'memory': {'current': 1024}, + 'cpu_info': {'vcpus': 1}, + 'disks': [{ + 'index': 0, 'size': 1, 'format': 'qcow2',

well, seems format is required now. What is it needed everywhere?

+ 'pool': {'name': '/plugins/kimchi/storagepools/default'}}], + 'source_media': UBUNTU_ISO} inst.templates_create(orig_params)

with RollbackContext() as rollback: @@ -1034,7 +1043,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')

@@ -1060,7 +1069,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')

@@ -1131,8 +1140,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..a30b0b1 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -18,7 +18,6 @@ # 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 copy import json import os import re @@ -107,7 +106,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 +133,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 +290,9 @@ class RestTests(unittest.TestCase):

def test_vm_lifecycle(self): # Create a Template - req = json.dumps({'name': 'test', 'disks': DISKS, + req = json.dumps({'name': 'test', 'source_media': fake_iso, 'icon': 'plugins/kimchi/images/icon-debian.png', - 'cdrom': 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,8 @@ 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,8 +953,9 @@ class RestTests(unittest.TestCase):

# Create template fails because SCSI volume is missing tmpl_params = { - 'name': 'test_fc_pool', 'cdrom': fake_iso, 'disks': [{'pool': { - 'name': '/plugins/kimchi/storagepools/scsi_fc_pool'}}]} + 'name': 'test_fc_pool', 'source_media': fake_iso, + 'disks': [{'pool': {'name': + '/plugins/kimchi/storagepools/scsi_fc_pool'}}]}

req = json.dumps(tmpl_params) resp = self.request('/plugins/kimchi/templates', req, 'POST') @@ -998,7 +998,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 +1039,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 +1110,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 +1148,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..d9e7f36 100644 --- a/tests/test_template.py +++ b/tests/test_template.py @@ -18,6 +18,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 json import os import psutil @@ -38,7 +39,7 @@ host = None port = None ssl_port = None cherrypy_port = None - +MOCK_ISO = "/tmp/mock.iso" DEFAULT_POOL = u'/plugins/kimchi/storagepools/default-pool'

@@ -53,6 +54,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(MOCK_ISO, True, '14.04', 'ubuntu')

def tearDownModule(): @@ -79,7 +81,7 @@ class TemplateTests(unittest.TestCase): self.assertEquals(400, resp.status)

# Create a template - t = {'name': 'test', 'cdrom': '/tmp/mock.iso'} + t = {'name': 'test', 'source_media': MOCK_ISO} req = json.dumps(t) resp = self.request('/plugins/kimchi/templates', req, 'POST') self.assertEquals(201, resp.status) @@ -117,22 +119,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': 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': MOCK_ISO, 'disks': [{ 'index': 0, 'size': 10, 'format': 'vmdk', 'pool': { 'name': DEFAULT_POOL}}]} req = json.dumps(t) @@ -149,7 +149,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': MOCK_ISO, 'memory': {'current': memory}} req = json.dumps(t) resp = self.request('/plugins/kimchi/templates', req, 'POST') @@ -158,7 +158,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': MOCK_ISO} req = json.dumps(t) resp = self.request('/plugins/kimchi/templates', req, 'POST') self.assertEquals(201, resp.status) @@ -328,7 +328,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': MOCK_ISO} req = json.dumps(t) resp = self.request('/plugins/kimchi/templates', req, 'POST') self.assertEquals(201, resp.status) @@ -364,7 +364,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': MOCK_ISO} req = json.dumps(t) resp = self.request('/plugins/kimchi/templates', req, 'POST') self.assertEquals(201, resp.status) @@ -410,13 +410,16 @@ class TemplateTests(unittest.TestCase): vol = vols[0]['name'] req = json.dumps({'disks': [{'volume': vol, 'pool': {'name': pool_uri}, - 'format': 'raw'}]}) + 'format': 'raw', + 'index': 1}]}) elif pool['type'] == 'logical': req = json.dumps({'disks': [{'pool': {'name': pool_uri}, - 'format': 'raw', 'size': 10}]}) + 'format': 'raw', 'size': 10, + 'index': 1}]}) else: req = json.dumps({'disks': [{'pool': {'name': pool_uri}, - 'format': 'qcow2', 'size': 10}]}) + 'format': 'qcow2', 'size': 10, + 'index': 1}]})

if req is not None: resp = self.request('/plugins/kimchi/templates/test', req, @@ -425,7 +428,7 @@ class TemplateTests(unittest.TestCase):

# Test disk template update with different pool pool_uri = u'/plugins/kimchi/storagepools/kīмсhīUnitTestDirPool' - disk_data = {'disks': [{'size': 5, 'format': 'qcow2', + disk_data = {'disks': [{'size': 5, 'format': 'qcow2', 'index': 0, 'pool': {'name': pool_uri}}]} req = json.dumps(disk_data) resp = self.request('/plugins/kimchi/templates/test', req, 'PUT') @@ -453,12 +456,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': MOCK_ISO, + 'networks': ['nat-network']} req = json.dumps(t) resp = self.request('/plugins/kimchi/templates', req, 'POST') self.assertEquals(201, resp.status) @@ -482,7 +481,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..d50dbd2 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,7 +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,51 +46,52 @@ 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} - t = VMTemplate(args) + args = {'name': 'test', 'source_media': self.iso} + t = VMTemplate(args, True) 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} - t = VMTemplate(args) + args = {'name': 'test', 'graphics': graphics, 'disks': DISKS, + 'source_media': self.iso} + t = VMTemplate(args, True) self.assertEquals(2, len(t.info['disks'])) self.assertEquals(graphics, t.info['graphics'])

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} - t = VMTemplate(args) + args = {'name': 'test', 'graphics': graphics, 'disks': DISKS, + 'source_media': self.iso} + t = VMTemplate(args, True) self.assertEquals(graphics, t.info['graphics'])

# Test specified type graphics = {'type': 'spice', 'listen': '127.0.0.1'} args['graphics'] = graphics - t = VMTemplate(args) + t = VMTemplate(args, True) self.assertEquals(graphics, t.info['graphics'])

# If no listen specified, test the default listen graphics = {'type': 'vnc'} args['graphics'] = graphics - t = VMTemplate(args) + t = VMTemplate(args, True) self.assertEquals(graphics['type'], t.info['graphics']['type']) self.assertEquals('127.0.0.1', t.info['graphics']['listen'])

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}, + True) 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,8 +110,9 @@ 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, - 'memory': {'current': (host_memory >> 10) - 512}}) + t = VMTemplate({'name': 'test-template', 'source_media': self.iso, + 'memory': {'current': (host_memory >> 10) - 512}}, + True) try: xml = t.to_vm_xml('test-vm', vm_uuid, graphics=graphics) except Exception as e: @@ -125,12 +128,13 @@ 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} - t = VMTemplate(args) + 'networks': ['foo'], 'source_media': self.iso, + 'graphics': graphics} + t = VMTemplate(args, True) 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'))