Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 04/07/2016 03:47 PM, Ramon Medeiros wrote:

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

diff --git a/API.json b/API.json index 6f0b484..ff505b1 100644 --- a/API.json +++ b/API.json @@ -468,11 +468,11 @@ "error": "KCHTMPL0011E" }, "memory": { "$ref": "#/kimchitype/memory" }, - "cdrom": { - "description": "Path for cdrom", - "type": "string", - "pattern": "^((/)|(http)[s]?:|[t]?(ftp)[s]?:)+.*$", - "error": "KCHTMPL0014E" + "source_media": { + "description": "Path for installation media (ISO, disk, remote ISO)", + "type" : "string", + "pattern" : "^((/)|(http)[s]?:|[t]?(ftp)[s]?:)+.*$", + "required": true }, "disks": { "description": "List of disks", @@ -497,12 +497,6 @@ "minimum": 1, "error": "KCHTMPL0022E" }, - "base": { - "description": "Base image of the disk", - "type": "string", - "pattern": "^/.+$", - "error": "KCHTMPL0023E" - }, "pool": { "description": "Storage pool information", "type": "object", 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 04/07/2016 03:47 PM, 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.

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

diff --git a/model/templates.py b/model/templates.py index 92705b6..65d04b7 100644 --- a/model/templates.py +++ b/model/templates.py @@ -19,23 +19,28 @@

import copy import libvirt +import magic import os import platform import psutil import stat +import urlparse

-from wok.exception import InvalidOperation, InvalidParameter +from wok.exception import InvalidOperation, InvalidParameter, MissingParameter from wok.exception import NotFoundError, OperationFailed from wok.utils import probe_file_permission_as_user, run_setfacl_set_attr from wok.xmlutils.utils import xpath_get_text

+from wok.plugins.kimchi import osinfo from wok.plugins.kimchi.config import get_kimchi_version from wok.plugins.kimchi.kvmusertests import UserTests from wok.plugins.kimchi.model.cpuinfo import CPUInfoModel from wok.plugins.kimchi.utils import pool_name_from_uri from wok.plugins.kimchi.vmtemplate import VMTemplate

- +DISK_TYPE = {"QEMU QCOW Image": "qcow2", + "data": "raw"} +ISO_TYPE = "ISO 9660 CD-ROM" # In PowerPC, memories must be aligned to 256 MiB PPC_MEM_ALIGN = 256 # Max memory 16TB for PPC and 4TiB for X (according to Red Hat), in KiB @@ -51,18 +56,11 @@ class TemplatesModel(object):

def create(self, params): name = params.get('name', '').strip() - iso = params.get('cdrom') - # check search permission - if iso and iso.startswith('/') and os.path.exists(iso): - st_mode = os.stat(iso).st_mode - if stat.S_ISREG(st_mode) or stat.S_ISBLK(st_mode): - user = UserTests().probe_user() - run_setfacl_set_attr(iso, user=user) - ret, excp = probe_file_permission_as_user(iso, user) - if ret is False: - raise InvalidParameter('KCHISO0008E', - {'filename': iso, 'user': user, - 'err': excp})

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

It is too early to verify the name here. Let's say user did not pass any name. So according to the line above it will be an empty string. You will check if a template with an empty string name already exists. Also after calling VMTemplate, a new name will be automatically generated BUT there will be no confirmation it is unique.

conn = self.conn.get() for net_name in params.get(u'networks', []): @@ -71,9 +69,46 @@ class TemplatesModel(object): except Exception: raise InvalidParameter("KCHTMPL0003E", {'network': net_name, 'template': name}) + + # no source_media argument: raise error + if params.get("source_media") is None: + raise MissingParameter('KCHTMPL0016E')

As you add source_media as a required parameter in API.json you don't need to verify it again as it will be always present at this point.

+ + path = params.pop("source_media") + + # not local image: set as remote ISO + if urlparse.urlparse(path).scheme in ["http", "https", "tftp", "ftp", + "ftps"]: + params["cdrom"] = path +

+ # image does not exists: raise error + if not os.path.exists(path): + raise InvalidParameter("Unable to find file %(path)s" % + {"path": path}) +

It needs to be a 'elif' otherwise you will raise an error for remote ISO.

+ # create magic object to discover file type + file_type = magic.open(magic.MAGIC_NONE) + file_type.load() + ftype = file_type.file(path) + + # cdrom + if ISO_TYPE in ftype: + params["cdrom"] = path + + # disk + else:

+ for types in DISK_TYPE.keys(): + if types in ftype: + # get default disk pool + params["disks"] = osinfo.lookup("unknow", + "unknow")["disks"] + params["disks"][0]["base"] = path +

Do not call osinfo here. You don't even know which OS it is in use and you are assuming 'unknow'. Only set the disk[0][base] to whatever you want. Something like: disks = params.get(disks, None) if disks is None: disks = [{'base': path}] else:; disks[0]['base'] = path

+ return self.validate_template_config(params) + + def validate_template_config(self, params): + # 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 +128,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})

You need to keep it here to ensure the name generated is unique.

session.store('template', name, t.info, get_kimchi_version()) except InvalidOperation: @@ -158,7 +191,7 @@ class TemplateModel(object):

temp = self.lookup(name) temp['name'] = clone_name - ident = self.templates.create(temp) + ident = self.templates.validate_template_config(temp)

As the validate_template_config is storing the data in the objecstore, I suggest to rename it to "save_template()"

return ident

def delete(self, name): @@ -207,9 +240,9 @@ class TemplateModel(object):

self.delete(name) try: - ident = self.templates.create(new_t) + ident = self.templates.validate_template_config(new_t) except: - ident = self.templates.create(old_t) + ident = self.templates.validate_template_config(old_t) raise return ident

On 04/07/2016 03:47 PM, 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 | 67 ++++++++++++++++++--------------------------- tests/test_rest.py | 35 +++++++++++------------ tests/test_template.py | 39 +++++++++++++------------- tests/test_vmtemplate.py | 3 +- 7 files changed, 77 insertions(+), 91 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..9f5fe83 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -135,12 +135,9 @@ class ModelTests(unittest.TestCase): inst.task_wait(task['id']) task = inst.task_lookup(task['id']) self.assertEquals('finished', task['status']) - 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} + params = {'name': 'test', 'source_media': UBUNTU_ISO} + inst.templates_create(params) rollback.prependDefer(inst.template_delete, 'test')

@@ -267,16 +264,12 @@ class ModelTests(unittest.TestCase):

# Create template based on IMG file tmpl_name = "img-tmpl" - pool_uri = "/plugins/kimchi/storagepools/default" 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, - "format": "qcow2", - "pool": {"name": pool_uri}}]} + "os_distro": "unknown", "os_version": "unknown", + "source_media": vol_path}

inst.templates_create(tmpl_info) rollback.prependDefer(inst.template_delete, tmpl_name) @@ -299,7 +292,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', 'source_media': UBUNTU_ISO} inst.templates_create(params) with RollbackContext() as rollback: params = {'name': 'kimchi-vnc', @@ -329,7 +322,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', 'source_media': UBUNTU_ISO} inst.templates_create(params) with RollbackContext() as rollback: params = {'name': 'kimchi-serial', @@ -350,7 +343,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', 'source_media': UBUNTU_ISO} inst.templates_create(params) rollback.prependDefer(inst.template_delete, 'test')

@@ -466,7 +459,7 @@ class ModelTests(unittest.TestCase): inst.task_wait(task_id)

vm_name = 'kimchi-cdrom' - params = {'name': 'test', 'disks': [], 'cdrom': UBUNTU_ISO} + params = {'name': 'test', 'source_media': UBUNTU_ISO}

Do not remove 'disks' from the above tests. Only change cdrom to source_media.

inst.templates_create(params) rollback.prependDefer(inst.template_delete, 'test') params = {'name': vm_name, @@ -476,7 +469,7 @@ class ModelTests(unittest.TestCase): rollback.prependDefer(inst.vm_delete, vm_name)

prev_count = len(inst.vmstorages_get_list(vm_name)) - self.assertEquals(1, prev_count) + self.assertEquals(2, prev_count)

Why did it change?

# Volume format with mismatched type raise error cdrom_args = {"type": "cdrom", "pool": pool, "vol": vol} @@ -509,8 +502,8 @@ class ModelTests(unittest.TestCase): iso_gen.construct_fake_iso(old_distro_iso, True, '4.8', 'rhel')

vm_name = 'kimchi-ide-bus-vm' - params = {'name': 'old_distro_template', 'disks': [], - 'cdrom': old_distro_iso} + params = {'name': 'old_distro_template', + 'source_media': old_distro_iso}

Do not remove 'disks'. Only replace cdrom to source_media.

inst.templates_create(params) rollback.prependDefer(inst.template_delete, 'old_distro_template') params = { @@ -535,7 +528,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', 'source_media': UBUNTU_ISO}

Same here.

inst.templates_create(params) rollback.prependDefer(inst.template_delete, 'test') params = {'name': vm_name, @@ -545,7 +538,7 @@ class ModelTests(unittest.TestCase): rollback.prependDefer(inst.vm_delete, vm_name)

prev_count = len(inst.vmstorages_get_list(vm_name)) - self.assertEquals(1, prev_count) + self.assertEquals(2, prev_count)

Same question I did above. Why the number of disks changed?

# dummy .iso files iso_path = os.path.join(TMP_DIR, 'existent.iso') @@ -624,9 +617,8 @@ 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', 'source_media': UBUNTU_ISO} +

Do not remove disks. Only change cdrom to source_media.

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

@@ -683,9 +675,7 @@ 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', 'source_media': UBUNTU_ISO}

Same here.

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

@@ -713,7 +703,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')

@@ -736,9 +727,7 @@ 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', 'source_media': UBUNTU_ISO} inst.templates_create(params)

And here.

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

@@ -758,7 +747,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 +782,10 @@ 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}, + 'source_media': UBUNTU_ISO}

And here.

inst.templates_create(orig_params)

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

And here.

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

@@ -1060,7 +1048,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}

And here.

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

@@ -1131,8 +1119,7 @@ class ModelTests(unittest.TestCase): with RollbackContext() as rollback: params = { 'name': 'test', - 'disks': [], - 'cdrom': UBUNTU_ISO, + 'source_media': UBUNTU_ISO, 'domain': 'test',

And here.

'arch': 'i686' } diff --git a/tests/test_rest.py b/tests/test_rest.py index d0d2fcf..5db44ce 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', 'source_media': fake_iso, + 'icon': 'plugins/kimchi/images/icon-debian.png'})

Do not remove disks from the test input. Only replace cdrom to source_media

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})

Same here.

resp = self.request('/plugins/kimchi/templates', req, 'POST') self.assertEquals(201, resp.status)

@@ -954,8 +952,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 +997,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 +1038,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 +1109,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 +1147,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..1b27130 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,24 +119,23 @@ 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': [{ - 'index': 0, 'size': 10, 'format': 'vmdk', 'pool': { + t = {'name': 'test-format', 'source_media': MOCK_ISO, 'disks': [{ + 'size': 10, 'format': 'vmdk', 'pool': { 'name': DEFAULT_POOL}}]} + req = json.dumps(t) resp = self.request('/plugins/kimchi/templates', req, 'POST') self.assertEquals(201, resp.status) @@ -149,7 +150,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 +159,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 +329,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 +365,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) @@ -442,6 +443,8 @@ class TemplateTests(unittest.TestCase): sorted(tmpl['disks'][0].values()))

def test_tmpl_integrity(self): + mock_iso2 = "/tmp/mock2.iso" + iso_gen.construct_fake_iso(mock_iso2, True, '14.04', 'ubuntu') # Create a network and a pool for testing template integrity net = {'name': u'nat-network', 'connection': 'nat'} self.request('/plugins/kimchi/networks', json.dumps(net), 'POST') @@ -453,12 +456,7 @@ 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_iso2, 'networks': ['nat-network']}

Do not remove disks. Only replace cdrom to source_media.

req = json.dumps(t) resp = self.request('/plugins/kimchi/templates', req, 'POST') self.assertEquals(201, resp.status) @@ -484,5 +482,6 @@ class TemplateTests(unittest.TestCase): self.assertEquals(400, resp.status)

# Verify the template + os.remove(mock_iso2) res = json.loads(self.request('/plugins/kimchi/templates/test').read()) - self.assertEquals(res['invalid']['cdrom'], ['/tmp/mock.iso']) + self.assertEquals(res['invalid']['cdrom'], [mock_iso2]) diff --git a/tests/test_vmtemplate.py b/tests/test_vmtemplate.py index eed58b0..503d34a 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)

Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 04/07/2016 03:47 PM, Ramon Medeiros wrote:

Signed-off-by: Ramon Medeiros <ramonn@linux.vnet.ibm.com> --- tests/test_model.py | 4 ++++ tests/test_template.py | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/tests/test_model.py b/tests/test_model.py index 9f5fe83..ca0e6f5 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -274,6 +274,10 @@ class ModelTests(unittest.TestCase): inst.templates_create(tmpl_info) rollback.prependDefer(inst.template_delete, tmpl_name)

+ # verify disk + tmpl = inst.template_lookup(tmpl_name) + self.assertEquals(vol_path, tmpl["disks"][0]["base"]) + params = {'name': 'kimchi-vm', 'template': '/plugins/kimchi/templates/img-tmpl'} task = inst.vms_create(params) diff --git a/tests/test_template.py b/tests/test_template.py index 1b27130..a6e58d6 100644 --- a/tests/test_template.py +++ b/tests/test_template.py @@ -93,7 +93,7 @@ class TemplateTests(unittest.TestCase): self.request('/plugins/kimchi/templates/test').read() ) self.assertEquals(sorted(tmpl.keys()), sorted(keys)) - + self.assertEquals(t['source_media'], tmpl["cdrom"]) disk_keys = ['index', 'pool', 'size', 'format'] disk_pool_keys = ['name', 'type'] self.assertEquals(sorted(tmpl['disks'][0].keys()), sorted(disk_keys))