Re: [Kimchi-devel] [PATCH v2 5/5] test_model: test creating iSCSI storage pool

On 12/22/2013 08:55 AM, Zhou Zheng Sheng wrote: > This patch adds iSCSI storage pool test to test_modle.test_storagepool. typo: test_modle

> It firstly checks if there exists an iSCSI target named > "iqn.2013-12.localhost.kimchiUnitTest" on local host. If not, skip the > test; if yes, continue to run the test already defined in > test_storagepool. > > On RedHat family distributions, libiscsi comes with a Python binding, > but not on other distributions. So in this patch it invokes iscsiadm to > check iSCSI target. > > Signed-off-by: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt; > --- > Makefile.am | 1 + > src/kimchi/Makefile.am | 1 + > src/kimchi/iscsi.py | 57 +++++++++++++++++++++++++++++++ > tests/test_model.py | 91 > ++++++++++++++++++++++++++++---------------------- > 4 files changed, 111 insertions(+), 39 deletions(-) > create mode 100644 src/kimchi/iscsi.py > > diff --git a/Makefile.am b/Makefile.am > index cbdc128..d832fcd 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -41,6 +41,7 @@ PEP8_WHITELIST = \ > src/kimchi/asynctask.py \ > src/kimchi/config.py.in \ > src/kimchi/disks.py \ > + src/kimchi/iscsi.py \ > src/kimchi/server.py \ > plugins/__init__.py \ > plugins/sample/__init__.py \ > diff --git a/src/kimchi/Makefile.am b/src/kimchi/Makefile.am > index 47a3bd2..02b6fee 100644 > --- a/src/kimchi/Makefile.am > +++ b/src/kimchi/Makefile.am > @@ -28,6 +28,7 @@ kimchi_PYTHON = \ > distroloader.py \ > exception.py \ > __init__.py \ > + iscsi.py \ > isoinfo.py \ > netinfo.py \ > network.py \ > diff --git a/src/kimchi/iscsi.py b/src/kimchi/iscsi.py > new file mode 100644 > index 0000000..96c77fa > --- /dev/null > +++ b/src/kimchi/iscsi.py It is only used for tests so move it under /tests

> @@ -0,0 +1,57 @@ > +# > +# Project Kimchi > +# > +# Copyright IBM, Corp. 2013 > +# > +# Authors: > +# Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt; > +# > +# This library is free software; you can redistribute it and/or > +# modify it under the terms of the GNU Lesser General Public > +# License as published by the Free Software Foundation; either > +# version 2.1 of the License, or (at your option) any later version. > +# > +# This library is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > +# Lesser General Public License for more details. > +# > +# 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-1301USA > + > +import subprocess > + 2 lines

> +from kimchi.exception import OperationFailed > + > + > +class IscsiAdmin(object): > + def __init__(self, host, port=None): > + self.portal = host + ("" if port is None else ":%s" % port) > + > + def _run_op(self, mode, target, op): > + iscsiadm = subprocess.Popen( > + ['iscsiadm', '--mode', mode, '--targetname', target, > + '--portal', self.portal, '--' + op], > + stdout=subprocess.PIPE, stderr=subprocess.PIPE) > + out, err = iscsiadm.communicate() > + if iscsiadm.returncode != 0: > + raise OperationFailed('Error executing iscsiadm: %s' % err) > + return out > + > + def login(self, target): > + self._run_op('node', target, 'login') > + > + def logout(self, target): > + self._run_op('node', target, 'logout') > + > + > +def validate_iscsi_target(target, host, port=None): > + adm = IscsiAdmin(host, port) > + try: > + adm.login(target) > + except OperationFailed: > + return False > + > + adm.logout(target) > + return True Let's do a python module with classes or functions - never both That way we can easily identify modules which implements instances types or use functions The function validate_iscsi_target can be into /tests/utils.py module.

> diff --git a/tests/test_model.py b/tests/test_model.py > index fb7d6dd..82fa57d 100644 > --- a/tests/test_model.py > +++ b/tests/test_model.py > @@ -34,6 +34,7 @@ import kimchi.model > import kimchi.objectstore > from kimchi.exception import * > from kimchi import netinfo > +from kimchi.iscsi import validate_iscsi_target > import utils > import iso_gen > > @@ -112,48 +113,60 @@ class ModelTests(unittest.TestCase): > def test_storagepool(self): > inst = kimchi.model.Model('qemu:///system', self.tmp_store) > > - with utils.RollbackContext() as rollback: > - path = '/tmp/kimchi-images' > - name = 'test-pool' > - if not os.path.exists(path): > - os.mkdir(path) > + poolDefs = [ > + {'type': 'dir', > + 'name': 'unitTestDirPool', > + 'path': '/tmp/kimchi-images'}, > + {'type': 'iscsi', > + 'name': 'unitTestISCSIPool', > + 'source': {'host': '127.0.0.1', > + 'target': > 'iqn.2013-12.localhost.kimchiUnitTest'}}] > > - pools = inst.storagepools_get_list() > - num = len(pools) + 1 > + for poolDef in poolDefs: > + with utils.RollbackContext() as rollback: > + path = poolDef.get('path') > + name = poolDef['name'] > + if not (path is None or os.path.exists(path)): > + os.mkdir(path) The backend should be able to create the path if it doesn't exist by pool.build()

> + > + if poolDef['type'] == 'iscsi': > + if not validate_iscsi_target(**poolDef['source']): > + continue > + > + pools = inst.storagepools_get_list() > + num = len(pools) + 1 > + > + inst.storagepools_create(poolDef) > + rollback.prependDefer(inst.storagepool_delete, name) > + > + pools = inst.storagepools_get_list() > + self.assertEquals(num, len(pools)) > + > + poolinfo = inst.storagepool_lookup(name) > + if path is not None: > + self.assertEquals(path, poolinfo['path']) > + self.assertEquals('inactive', poolinfo['state']) > + if poolinfo['type'] == 'dir': > + self.assertEquals(True, poolinfo['autostart']) > + else: > + self.assertEquals(False, poolinfo['autostart']) > + > + inst.storagepool_activate(name) > + rollback.prependDefer(inst.storagepool_deactivate, name) > > - args = {'name': name, > - 'path': path, > - 'type': 'dir'} > - inst.storagepools_create(args) > - rollback.prependDefer(inst.storagepool_delete, name) > - > - pools = inst.storagepools_get_list() > - self.assertEquals(num, len(pools)) > - > - poolinfo = inst.storagepool_lookup(name) > - self.assertEquals(path, poolinfo['path']) > - self.assertEquals('inactive', poolinfo['state']) > - if poolinfo['type'] == 'dir': > - self.assertEquals(True, poolinfo['autostart']) > - else: > - self.assertEquals(False, poolinfo['autostart']) > - > - inst.storagepool_activate(name) > - rollback.prependDefer(inst.storagepool_deactivate, name) > - > - poolinfo = inst.storagepool_lookup(name) > - self.assertEquals('active', poolinfo['state']) > - > - autostart = poolinfo['autostart'] > - ori_params = {'autostart': True} if autostart else > {'autostart': False} > - for i in [True, False]: > - params = {'autostart': i} > - inst.storagepool_update(name, params) > - rollback.prependDefer(inst.storagepool_update, name, > - ori_params) > poolinfo = inst.storagepool_lookup(name) > - self.assertEquals(i, poolinfo['autostart']) > - inst.storagepool_update(name, ori_params) > + self.assertEquals('active', poolinfo['state']) > + > + autostart = poolinfo['autostart'] > + ori_params = {'autostart': True} if autostart else > {'autostart': False} > + for i in [True, False]: > + params = {'autostart': i} > + inst.storagepool_update(name, params) > + rollback.prependDefer(inst.storagepool_update, name, > + ori_params) > + poolinfo = inst.storagepool_lookup(name) > + self.assertEquals(i, poolinfo['autostart']) > + inst.storagepool_update(name, ori_params) > > pools = inst.storagepools_get_list() > self.assertIn('default', pools)