Backend and tests need some adjustments to perform all tasks and tests after remove support to 'storagepool' in Templates. Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo@linux.vnet.ibm.com> Signed-off-by: Aline Manera <alinefm@linux.vnet.ibm.com> --- mockmodel.py | 2 +- model/templates.py | 80 ++++++------------------- tests/test_model.py | 59 +++++++++--------- tests/test_rest.py | 20 ++++--- tests/test_template.py | 33 ++++++---- tests/test_vmtemplate.py | 9 ++- vmtemplate.py | 37 +++++++++--- 7 files changed, 119 insertions(+), 121 deletions(-) diff --git a/mockmodel.py b/src/wok/plugins/kimchi/mockmodel.py index 2e58724..8263207 100644 --- a/mockmodel.py +++ b/mockmodel.py @@ -75,7 +75,7 @@ class MockModel(Model): # test:///default driver defaults = dict(osinfo.defaults) defaults.update(mockmodel_defaults) - defaults['disks'][0]['pool'] = DEFAULT_POOL + defaults['disks'][0]['pool'] = {'name': DEFAULT_POOL} osinfo.defaults = dict(defaults) self._mock_vgs = MockVolumeGroups() diff --git a/model/templates.py b/src/wok/plugins/kimchi/model/templates.py index 7b1412b..84cdd02 100644 --- a/model/templates.py +++ b/model/templates.py @@ -22,7 +22,7 @@ import libvirt import os import stat -from wok.exception import InvalidOperation, InvalidParameter, MissingParameter +from wok.exception import InvalidOperation, InvalidParameter 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 @@ -34,43 +34,6 @@ from wok.plugins.kimchi.utils import pool_name_from_uri from wok.plugins.kimchi.vmtemplate import VMTemplate -def _check_disks_params(params, conn, objstore): - if 'disks' not in params: - return - - basic_disk = ['index', 'format', 'pool', 'size'] - ro_disk = ['index', 'format', 'pool', 'volume'] - base_disk = ['index', 'base', 'pool'] - for disk in params['disks']: - keys = sorted(disk.keys()) - if ((keys == sorted(basic_disk)) or (keys == sorted(ro_disk)) or - (keys == sorted(base_disk))): - pass - else: - raise MissingParameter('KCHTMPL0028E') - - if disk.get('pool', {}).get('name') is None: - raise MissingParameter('KCHTMPL0028E') - - pool_uri = disk['pool']['name'] - try: - pool_name = pool_name_from_uri(pool_uri) - conn.get().storagePoolLookupByName(pool_name.encode("utf-8")) - except Exception: - raise InvalidParameter("KCHTMPL0004E", - {'pool': pool_uri, - 'template': pool_name}) - if 'volume' in disk: - kwargs = {'conn': conn, 'objstore': objstore} - storagevolumes = __import__( - "wok.plugins.kimchi.model.storagevolumes", fromlist=['']) - pool_volumes = storagevolumes.StorageVolumesModel( - **kwargs).get_list(pool_name) - if disk['volume'] not in pool_volumes: - raise InvalidParameter("KCHTMPL0019E", {'pool': pool_name, - 'volume': disk['volume']}) - - class TemplatesModel(object): def __init__(self, **kargs): self.objstore = kargs['objstore'] @@ -91,9 +54,6 @@ class TemplatesModel(object): {'filename': iso, 'user': user, 'err': excp}) - # Check disks information - _check_disks_params(params, self.conn, self.objstore) - cpu_info = params.get('cpu_info') if cpu_info: topology = cpu_info.get('topology') @@ -121,6 +81,15 @@ class TemplatesModel(object): # Checkings will be done while creating this class, so any exception # will be raised here t = LibvirtVMTemplate(params, scan=True, conn=self.conn) + + # Validate volumes + for disk in t.info.get('disks'): + volume = disk.get('volume') + # volume can be None + if 'volume' in disk.keys(): + self.template_volume_validate(volume, disk['pool']) + + # Store template on objectstore name = params['name'] try: with self.objstore as session: @@ -141,10 +110,8 @@ class TemplatesModel(object): def template_volume_validate(self, volume, pool): kwargs = {'conn': self.conn, 'objstore': self.objstore} - pool_type = xpath_get_text(pool.XMLDesc(0), "/pool/@type")[0] - pool_name = unicode(pool.name(), 'utf-8') - - if pool_type in ['iscsi', 'scsi']: + pool_name = pool_name_from_uri(pool['name']) + if pool['type'] in ['iscsi', 'scsi']: if not volume: raise InvalidParameter("KCHTMPL0018E") @@ -164,18 +131,14 @@ class TemplateModel(object): self.templates = TemplatesModel(**kargs) @staticmethod - def get_template(name, objstore, conn, overrides=None): + def get_template(name, objstore, conn, overrides={}): with objstore as session: params = session.get('template', name) - if overrides: - if 'storagepool' in overrides: - for i, disk in enumerate(params['disks']): - params['disks'][i]['pool']['name'] = \ - overrides['storagepool'] - del overrides['storagepool'] - params.update(overrides) - - _check_disks_params(params, conn, objstore) + if overrides and 'storagepool' in overrides: + for i, disk in enumerate(params['disks']): + params['disks'][i]['pool']['name'] = overrides['storagepool'] + del overrides['storagepool'] + params.update(overrides) return LibvirtVMTemplate(params, False, conn) def lookup(self, name): @@ -212,9 +175,6 @@ class TemplateModel(object): new_t = copy.copy(old_t) new_t.update(params) - # Check disks information - _check_disks_params(new_t, self.conn, self.objstore) - if not self._validate_updated_cpu_params(new_t): raise InvalidParameter('KCHTMPL0025E') @@ -251,9 +211,7 @@ class LibvirtVMTemplate(VMTemplate): self.conn = conn VMTemplate.__init__(self, args, scan) - def _storage_validate(self, pool_uri=None): - if pool_uri is None: - pool_uri = self.info['storagepool'] + def _storage_validate(self, pool_uri): pool_name = pool_name_from_uri(pool_uri) try: conn = self.conn.get() diff --git a/tests/test_model.py b/src/wok/plugins/kimchi/tests/test_model.py index c5407d6..caacd91 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -39,7 +39,6 @@ from wok.xmlutils.utils import xpath_get_text from wok.plugins.kimchi import netinfo from wok.plugins.kimchi import osinfo -from wok.plugins.kimchi.config import get_kimchi_version from wok.plugins.kimchi.config import kimchiPaths as paths from wok.plugins.kimchi.model import model from wok.plugins.kimchi.model.libvirtconnection import LibvirtConnection @@ -77,13 +76,13 @@ def tearDownModule(): def _setDiskPoolDefault(): - osinfo.defaults['disks'][0]['pool'] = \ - '/plugins/kimchi/storagepools/default' + osinfo.defaults['disks'][0]['pool'] = { + 'name': '/plugins/kimchi/storagepools/default'} def _setDiskPoolDefaultTest(): - osinfo.defaults['disks'][0]['pool'] = \ - '/plugins/kimchi/storagepools/default-pool' + osinfo.defaults['disks'][0]['pool'] = { + 'name': '/plugins/kimchi/storagepools/default-pool'} class ModelTests(unittest.TestCase): @@ -138,8 +137,9 @@ class ModelTests(unittest.TestCase): vol = inst.storagevolume_lookup(u'default', vol_params['name']) params = {'name': 'test', 'disks': [{'base': vol['path'], - 'size': 1}], - 'cdrom': UBUNTU_ISO} + 'size': 1, 'pool': { + 'name': '/plugins/kimchi/storagepools/default'}}], + 'cdrom': UBUNTU_ISO} inst.templates_create(params) rollback.prependDefer(inst.template_delete, 'test') @@ -264,24 +264,20 @@ class ModelTests(unittest.TestCase): self.assertEquals('finished', inst.task_lookup(task_id)['status']) vol_path = inst.storagevolume_lookup('default', vol)['path'] - # Hack the model objstore to add a new template - # It is needed as the image file must be a bootable image when - # using model - # As it is difficult to create one on test runtime, inject a - # template with an empty image file to the objstore to test the - # feature + # Create template based on IMG file tmpl_name = "img-tmpl" - tmpl_info = {"cpus": 1, "cdrom": "", + pool_uri = "/plugins/kimchi/storagepools/default" + tmpl_info = {"cpus": 1, "cdrom": "", "name": tmpl_name, "graphics": {"type": "vnc", "listen": "127.0.0.1"}, "networks": ["default"], "memory": 1024, "folder": [], "icon": "images/icon-vm.png", "os_distro": "unknown", "os_version": "unknown", - "disks": [{"base": vol_path, "size": 10, "pool": { - "name": "/plugins/kimchi/storagepools/default"}}]} + "disks": [{"base": vol_path, "size": 10, + "format": "qcow2", + "pool": {"name": pool_uri}}]} - with inst.objstore as session: - session.store('template', tmpl_name, tmpl_info, - get_kimchi_version()) + inst.templates_create(tmpl_info) + rollback.prependDefer(inst.template_delete, tmpl_name) params = {'name': 'kimchi-vm', 'template': '/plugins/kimchi/templates/img-tmpl'} @@ -606,8 +602,9 @@ class ModelTests(unittest.TestCase): inst = model.Model(objstore_loc=self.tmp_store) with RollbackContext() as rollback: - params = {'name': 'test', 'disks': [{'size': 1}], - 'cdrom': UBUNTU_ISO} + params = {'name': 'test', 'disks': [{'size': 1, 'pool': { + 'name': '/plugins/kimchi/storagepools/default'}}], + 'cdrom': UBUNTU_ISO} inst.templates_create(params) rollback.prependDefer(inst.template_delete, 'test') @@ -664,8 +661,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}], - 'cdrom': UBUNTU_ISO} + params = {'name': 'test', 'disks': [{'size': 1, 'pool': { + 'name': '/plugins/kimchi/storagepools/default'}}], + 'cdrom': UBUNTU_ISO} inst.templates_create(params) rollback.prependDefer(inst.template_delete, 'test') @@ -690,9 +688,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}], - 'cdrom': UBUNTU_ISO} + params = {'name': 'test', 'disks': [{ + 'size': 1, 'format': user_vol, + 'pool': {'name': '/plugins/kimchi/storagepools/default'}}], + 'cdrom': UBUNTU_ISO} inst.templates_create(params) rollback.prependDefer(inst.template_delete, 'test') @@ -715,8 +714,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}], 'cdrom': UBUNTU_ISO} + params = {'name': 'test', 'disks': [{'size': 1, 'pool': { + 'name': '/plugins/kimchi/storagepools/default'}}], + 'cdrom': UBUNTU_ISO} inst.templates_create(params) rollback.prependDefer(inst.template_delete, 'test') @@ -770,7 +770,8 @@ class ModelTests(unittest.TestCase): # only supports this format orig_params = {'name': 'test', 'memory': 1024, 'cpus': 1, 'cdrom': UBUNTU_ISO, - 'disks': [{'size': 1, 'format': 'qcow2'}]} + 'disks': [{'size': 1, 'format': 'qcow2', 'pool': { + 'name': '/plugins/kimchi/storagepools/default'}}]} inst.templates_create(orig_params) with RollbackContext() as rollback: diff --git a/tests/test_rest.py b/src/wok/plugins/kimchi/tests/test_rest.py index 2947df2..0305e82 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.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 copy import json import os import re @@ -47,6 +48,9 @@ ssl_port = None cherrypy_port = None fake_iso = '/tmp/fake.iso' +DISKS = [{'size': 10, 'format': 'qcow2', 'index': 0, 'pool': { + 'name': '/plugins/kimchi/storagepools/default-pool'}}] + def setUpModule(): global test_server, model, host, port, ssl_port, cherrypy_port @@ -267,7 +271,7 @@ class RestTests(unittest.TestCase): def test_vm_lifecycle(self): # Create a Template - req = json.dumps({'name': 'test', 'disks': [{'size': 1}], + req = json.dumps({'name': 'test', 'disks': DISKS, 'icon': 'plugins/kimchi/images/icon-debian.png', 'cdrom': fake_iso}) resp = self.request('/plugins/kimchi/templates', req, 'POST') @@ -291,7 +295,7 @@ class RestTests(unittest.TestCase): + '%s-0.img' resp = self.request(vol_uri % vm['uuid']) vol = json.loads(resp.read()) - self.assertEquals(1 << 30, vol['capacity']) + self.assertEquals(10 << 30, vol['capacity']) self.assertEquals(['test-vm'], vol['used_by']) # verify if poweroff command returns correct status @@ -868,7 +872,7 @@ class RestTests(unittest.TestCase): def test_vm_customise_storage(self): # Create a Template req = json.dumps({'name': 'test', 'cdrom': fake_iso, - 'disks': [{'size': 1}]}) + 'disks': DISKS}) resp = self.request('/plugins/kimchi/templates', req, 'POST') self.assertEquals(201, resp.status) @@ -905,7 +909,7 @@ class RestTests(unittest.TestCase): % vm_info['uuid'] resp = self.request(vol_uri) vol = json.loads(resp.read()) - self.assertEquals(1 << 30, vol['capacity']) + self.assertEquals(10 << 30, vol['capacity']) # Delete the VM resp = self.request('/plugins/kimchi/vms/test-vm', '{}', 'DELETE') @@ -943,8 +947,8 @@ class RestTests(unittest.TestCase): ) lun_name = json.loads(resp.read())[0]['name'] pool_name = tmpl_params['disks'][0]['pool']['name'] - tmpl_params['disks'] = [{'index': 0, 'volume': lun_name, - 'pool': {'name': pool_name}}] + tmpl_params['disks'] = [{'index': 0, 'volume': lun_name, 'pool': { + 'name': pool_name}, 'format': 'raw'}] req = json.dumps(tmpl_params) resp = self.request('/plugins/kimchi/templates', req, 'POST') self.assertEquals(201, resp.status) @@ -1016,7 +1020,9 @@ class RestTests(unittest.TestCase): # Create a Template mock_base = '/tmp/mock.img' open(mock_base, 'w').close() - req = json.dumps({'name': 'test', 'disks': [{'base': mock_base}]}) + disks = copy.deepcopy(DISKS) + disks[0]['base'] = mock_base + req = json.dumps({'name': 'test', 'disks': disks}) resp = self.request('/plugins/kimchi/templates', req, 'POST') self.assertEquals(201, resp.status) diff --git a/tests/test_template.py b/src/wok/plugins/kimchi/tests/test_template.py index 7bd856e..0cc5ac8 100644 --- a/tests/test_template.py +++ b/tests/test_template.py @@ -120,16 +120,18 @@ class TemplateTests(unittest.TestCase): # Create an image based template open('/tmp/mock.img', 'w').close() - t = {'name': 'test_img_template', - 'disks': [{'base': '/tmp/mock.img'}]} + t = {'name': 'test_img_template', 'disks': [{ + 'base': '/tmp/mock.img', 'format': 'qcow2', + 'pool': {'name': DEFAULT_POOL}, 'size': 1}]} 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'}]} + t = {'name': 'test-format', 'cdrom': '/tmp/mock.iso', 'disks': [{ + 'index': 0, '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) @@ -214,10 +216,11 @@ class TemplateTests(unittest.TestCase): self.assertEquals(update_tmpl['cdrom'], cdrom_data['cdrom']) # Update disks - disk_data = {'disks': [{'index': 0, 'size': 10, 'format': 'raw', - 'pool': DEFAULT_POOL}, + disk_data = {'disks': [{'index': 0, 'size': 10, + 'format': 'raw', 'pool': { + 'name': DEFAULT_POOL}}, {'index': 1, 'size': 20, 'format': 'qcow2', - 'pool': DEFAULT_POOL}]} + 'pool': {'name': DEFAULT_POOL}}]} resp = self.request(new_tmpl_uri, json.dumps(disk_data), 'PUT') self.assertEquals(200, resp.status) resp = self.request(new_tmpl_uri) @@ -236,7 +239,7 @@ class TemplateTests(unittest.TestCase): 'qed', 'raw', 'vmdk', 'vpc'] for disk_type in disk_types: disk_data = {'disks': [{'index': 0, 'format': disk_type, - 'size': 10, 'pool': DEFAULT_POOL}]} + 'size': 10, 'pool': {'name': DEFAULT_POOL}}]} resp = self.request(new_tmpl_uri, json.dumps(disk_data), 'PUT') self.assertEquals(200, resp.status) @@ -350,9 +353,14 @@ class TemplateTests(unittest.TestCase): if len(vols) > 0: vol = vols[0]['name'] req = json.dumps({'disks': [{'volume': vol, - 'pool': {'name': pool_uri}}]}) + 'pool': {'name': pool_uri}, + 'format': 'raw'}]}) + elif pool['type'] == 'logical': + req = json.dumps({'disks': [{'pool': {'name': pool_uri}, + 'format': 'raw', 'size': 10}]}) else: - req = json.dumps({'disks': [{'pool': {'name': pool_uri}}]}) + req = json.dumps({'disks': [{'pool': {'name': pool_uri}, + 'format': 'qcow2', 'size': 10}]}) if req is not None: resp = self.request('/plugins/kimchi/templates/test', req, @@ -362,7 +370,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', - 'pool': pool_uri}]} + 'pool': {'name': pool_uri}}]} req = json.dumps(disk_data) resp = self.request('/plugins/kimchi/templates/test', req, 'PUT') self.assertEquals(200, resp.status) @@ -393,7 +401,8 @@ class TemplateTests(unittest.TestCase): 'networks': ['nat-network'], 'disks': [{'pool': { 'name': '/plugins/kimchi/storagepools/dir-pool'}, - 'size': 2}]} + 'size': 2, + 'format': 'qcow2'}]} req = json.dumps(t) resp = self.request('/plugins/kimchi/templates', req, 'POST') self.assertEquals(201, resp.status) diff --git a/tests/test_vmtemplate.py b/src/wok/plugins/kimchi/tests/test_vmtemplate.py index 0bca215..79e3b52 100644 --- a/tests/test_vmtemplate.py +++ b/tests/test_vmtemplate.py @@ -26,6 +26,11 @@ from wok.xmlutils.utils import xpath_get_text from wok.plugins.kimchi.osinfo import get_template_default from wok.plugins.kimchi.vmtemplate import VMTemplate +DISKS = [{'size': 10, 'format': 'raw', 'index': 0, 'pool': {'name': + '/plugins/kimchi/storagepools/default-pool'}}, + {'size': 5, 'format': 'qcow2', 'index': 1, 'pool': {'name': + '/plugins/kimchi/storagepools/default-pool'}}] + class VMTemplateTests(unittest.TestCase): def setUp(self): @@ -53,7 +58,7 @@ class VMTemplateTests(unittest.TestCase): def test_construct_overrides(self): graphics = {'type': 'spice', 'listen': '127.0.0.1'} - args = {'name': 'test', 'disks': [{'size': 10}, {'size': 20}], + args = {'name': 'test', 'disks': DISKS, 'graphics': graphics, "cdrom": self.iso} t = VMTemplate(args) self.assertEquals(2, len(t.info['disks'])) @@ -62,7 +67,7 @@ class VMTemplateTests(unittest.TestCase): def test_specified_graphics(self): # Test specified listen graphics = {'type': 'vnc', 'listen': '127.0.0.1'} - args = {'name': 'test', 'disks': [{'size': 10}, {'size': 20}], + args = {'name': 'test', 'disks': DISKS, 'graphics': graphics, 'cdrom': self.iso} t = VMTemplate(args) self.assertEquals(graphics, t.info['graphics']) diff --git a/vmtemplate.py b/src/wok/plugins/kimchi/vmtemplate.py index 62d0e1a..b90f221 100644 --- a/vmtemplate.py +++ b/vmtemplate.py @@ -76,23 +76,40 @@ class VMTemplate(object): graphics.update(graph_args) args['graphics'] = graphics + default_disk = self.info['disks'][0] # Override template values according to 'args' self.info.update(args) - - # Find pool type for each disk disks = self.info.get('disks') - for disk in disks: - pool_name = disk.get('pool', {}).get('name') - if pool_name is None: - raise MissingParameter('KCHTMPL0029E') + + basic_disk = ['index', 'format', 'pool', 'size'] + ro_disk = ['index', 'format', 'pool', 'volume'] + base_disk = ['index', 'base', 'pool', 'size', 'format'] + + for index, disk in enumerate(disks): + disk_info = dict(default_disk) pool_type = self._get_storage_type(disk['pool']['name']) - disk['pool']['type'] = pool_type + if pool_type in ['iscsi', 'scsi']: + disk_info = {'index': 0, 'format': 'raw', 'volume': None} + + disk_info.update(disk) + pool_name = disk_info.get('pool', {}).get('name') + if pool_name is None: + raise MissingParameter('KCHTMPL0028E') + + keys = sorted(disk_info.keys()) + if ((keys != sorted(basic_disk)) and (keys != sorted(ro_disk)) and + (keys != sorted(base_disk))): + raise MissingParameter('KCHTMPL0028E') if pool_type in ['logical', 'iscsi', 'scsi']: - if disk['format'] != 'raw': + if disk_info['format'] != 'raw': raise InvalidParameter('KCHTMPL0029E') + disk_info['pool']['type'] = pool_type + disk_info['index'] = disk_info.get('index', index) + self.info['disks'][index] = disk_info + def _get_os_info(self, args, scan): distro = version = 'unknown' @@ -406,7 +423,9 @@ class VMTemplate(object): return xml def validate(self): - self._storage_validate() + for disk in self.info.get('disks'): + pool_uri = disk.get('pool', {}).get('name') + self._storage_validate(pool_uri) self._network_validate() self._iso_validate() -- 2.1.0