[Kimchi-devel] [PATCH] Fix get_storageservers API and UI retrieval of storage servers.

Thu Apr 10 23:18:38 UTC 2014

Tested and it worked fine

Reviewed-by: Rodrigo Trujillo <rodrigo.trujillo at linux.vnet.ibm.com>
Tested-by: Rodrigo Trujillo <rodrigo.trujillo at linux.vnet.ibm.com>


On 04/10/2014 01:14 PM, Christy Perez wrote:
> This issue was discovered when it was realized that the list of NFS
> servers currently on the system was not being displayed in a drop-down
> for the server IP/name while adding a new NFS server.
>
> The UI was calling the getstorageservers API and passing a filter of netfs, but
> the StorageServers class was referencing a non-existent object. This threw an
> exception that was not passed up as an error, and an empty list was returned
> every time.
>
> Additionally, a change to the STORAGE_SOURCES dictionary was causing errors
> when the StorageServersModel::lookup() tried to access a non-existent key in
> in the new scsi value. This is seen if a scsi pool exists on a system.
>
> To allow the API to work, and sucessfully retrieve only *server* objects, this
> patch makes the following changes:
>
> 1. Add a STORAGE_SERVERS list to storageservers.py containing only valid
>     server types.
>     The API doc states that currently only netfs is supported.
> 2. Since storagetargets correlates to an individual server (see the
>     API doc for storagetargets) use the new STORAGE_SERVERS list instead
>     of the sources ones.
> 3. Move the STORAGE_SOURCES dictionary into storagepools.py, since it should
>     cover all pool types. In the future, it should be expanded to include
>     iscsi, netfs, dir, scsi fc_host, etc.
> 4. Updates mockmodel to incorporate the new STORAGE_SERVERS list.
>
> To test, create different types of pools, one nfs, and use:
>
> $ curl -k -u user:pass -H 'Content-type: applicatiojson' -H 'Accept: application/json' https://localhost:8001/storageservers?_target_type=netfs
>
> and
>
> $ curl -k -u user:pass -H 'Content-type: applicatiojson' -H 'Accept: application/json' https://localhost:8001/storageservers
>
> Or create at least one NFS pool, then when creating a second NFS pool, the
>    names/IPs of the others will appear in the UI's server field called
>    "NFS Server IP"
>
> Signed-off-by: Christy Perez <christy at linux.vnet.ibm.com>
> ---
>   src/kimchi/mockmodel.py            |  5 +++--
>   src/kimchi/model/storagepools.py   |  3 ++-
>   src/kimchi/model/storageservers.py | 16 +++++++++-------
>   src/kimchi/model/storagetargets.py |  4 ++--
>   4 files changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py
> index dbdd57e..fe02abe 100644
> --- a/src/kimchi/mockmodel.py
> +++ b/src/kimchi/mockmodel.py
> @@ -46,7 +46,8 @@ from kimchi.config import config as kconfig
>   from kimchi.distroloader import DistroLoader
>   from kimchi.exception import InvalidOperation, InvalidParameter
>   from kimchi.exception import MissingParameter, NotFoundError, OperationFailed
> -from kimchi.model.storagepools import ISO_POOL_NAME, STORAGE_SOURCES
> +from kimchi.model.storagepools import ISO_POOL_NAME
> +from kimchi.model.storageservers import STORAGE_SERVERS
>   from kimchi.model.utils import get_vm_name
>   from kimchi.model.vms import VM_STATIC_UPDATE_PARAMS
>   from kimchi.objectstore import ObjectStore
> @@ -503,7 +504,7 @@ class MockModel(object):
>
>       def storageservers_get_list(self, _target_type=None):
>           # FIXME: This needs to be updted when adding new storage server support
> -        target_type = STORAGE_SOURCES.keys() \
> +        target_type = STORAGE_SERVERS \
>               if not _target_type else [_target_type]
>           pools = self.storagepools_get_list()
>           server_list = []
> diff --git a/src/kimchi/model/storagepools.py b/src/kimchi/model/storagepools.py
> index 5af33b7..0f673bb 100644
> --- a/src/kimchi/model/storagepools.py
> +++ b/src/kimchi/model/storagepools.py
> @@ -36,6 +36,8 @@ POOL_STATE_MAP = {0: 'inactive',
>                     3: 'degraded',
>                     4: 'inaccessible'}
>
> +# Types of pools supported
> +# FIXME: Add 'dir', 'logical' 'iscsi', 'fc_host' ?
>   STORAGE_SOURCES = {'netfs': {'addr': '/pool/source/host/@name',
>                                'path': '/pool/source/dir/@path'},
>                      'scsi': {'adapter_type': '/pool/source/adapter/@type',
> @@ -207,7 +209,6 @@ class StoragePoolModel(object):
>                   source[key] = ""
>               else:
>                   source[key] = res
> -
>           return source
>
>       def _nfs_status_online(self, pool, poolArgs=None):
> diff --git a/src/kimchi/model/storageservers.py b/src/kimchi/model/storageservers.py
> index 167fedd..c9dfb25 100644
> --- a/src/kimchi/model/storageservers.py
> +++ b/src/kimchi/model/storageservers.py
> @@ -18,24 +18,26 @@
>   # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301 USA
>
>   from kimchi.exception import NotFoundError
> -from kimchi.model.storagepools import StoragePoolModel, STORAGE_SOURCES
> +from kimchi.model.storagepools import StoragePoolModel, StoragePoolsModel
> +
> +# Types of remote storage servers supported
> +# FIXME: Add iscsi?
> +STORAGE_SERVERS = ['netfs']
>
>
>   class StorageServersModel(object):
>       def __init__(self, **kargs):
>           self.conn = kargs['conn']
>           self.pool = StoragePoolModel(**kargs)
> +        self.pools = StoragePoolsModel(**kargs)
>
>       def get_list(self, _target_type=None):
>           if not _target_type:
> -            target_type = STORAGE_SOURCES.keys()
> +            target_type = STORAGE_SERVERS
>           else:
>               target_type = [_target_type]
> -        pools = self.pools.get_list()
>
> -        conn = self.conn.get()
> -        pools = conn.listStoragePools()
> -        pools += conn.listDefinedStoragePools()
> +        pools = self.pools.get_list()
>
>           server_list = []
>           for pool in pools:
> @@ -64,7 +66,7 @@ class StorageServerModel(object):
>           for pool in pools:
>               try:
>                   pool_info = self.pool.lookup(pool)
> -                if (pool_info['source'] and
> +                if (pool_info['type'] in STORAGE_SERVERS and
>                           pool_info['source']['addr'] == server):
>                       return dict(host=server)
>               except NotFoundError:
> diff --git a/src/kimchi/model/storagetargets.py b/src/kimchi/model/storagetargets.py
> index e090d84..caa8dbe 100644
> --- a/src/kimchi/model/storagetargets.py
> +++ b/src/kimchi/model/storagetargets.py
> @@ -23,7 +23,7 @@ from lxml import objectify
>   from lxml.builder import E
>
>   from kimchi.model.config import CapabilitiesModel
> -from kimchi.model.storagepools import STORAGE_SOURCES
> +from kimchi.model.storageservers import STORAGE_SERVERS
>   from kimchi.utils import kimchi_log, patch_find_nfs_target
>
>
> @@ -36,7 +36,7 @@ class StorageTargetsModel(object):
>           target_list = list()
>
>           if not _target_type:
> -            target_types = STORAGE_SOURCES.keys()
> +            target_types = STORAGE_SERVERS
>           else:
>               target_types = [_target_type]
>