[Kimchi-devel] [PATCH] [Kimchi] Kimchi kills Wokd due to sys.exit() calls in files networks.py and storagepools.py

[Aline Manera]

Hi Ramon,

On 07/23/2016 12:34 PM, Ramon Medeiros wrote:
> Would be nice to create a custom exception, like happens on 
> src/wok/exception.py. You can also set the http status

The exceptions in src/wok/exception.py depends on cherrypy server to get 
the error message according to the message code, *but* at this point, 
when Wok is initializing the plugin, the cherrypy server is not up yet.
So if we try to use an exception type which depends on cherrypy it will 
return <code>:<code> as it will not be able to get the message.

This error message will not be displayed on UI. It will be an error on 
Wok start up to alert the sysadmin the Kimchi plugin was not loaded due 
an failure.

>
> On 07/22/2016 08:14 PM, bianca at linux.vnet.ibm.com wrote:
>> From: Bianca Carvalho <bianca at linux.vnet.ibm.com>
>>
>> Removed all sys.exit(1) calls from both files to avoid killing Wokd.
>> Also added an Exception to not load kimchi plugin in case network or
>> storagepool does not exist or is not active.
>>
>> Signed-off-by: Bianca Carvalho <bianca at linux.vnet.ibm.com>
>> ---
>>   model/networks.py     | 16 +++++++---------
>>   model/storagepools.py | 19 ++++++++-----------
>>   2 files changed, 15 insertions(+), 20 deletions(-)
>>
>> diff --git a/model/networks.py b/model/networks.py
>> index 4b722d3..35431d4 100644
>> --- a/model/networks.py
>> +++ b/model/networks.py
>> @@ -20,7 +20,6 @@
>>   import copy
>>   import ipaddr
>>   import libvirt
>> -import sys
>>   import time
>>   from libvirt import VIR_INTERFACE_XML_INACTIVE
>>
>> @@ -59,19 +58,19 @@ class NetworksModel(object):
>>           networks = list(set(tmpl_defaults['networks']))
>>           conn = self.conn.get()
>>
>> -        error_msg = ("Please, check the configuration in 
>> %s/template.conf to "
>> -                     "ensure it lists only valid networks." %
>> -                     kimchiPaths.sysconf_dir)
>> -
>>           for net_name in networks:
>> +            error_msg = ("Network %s does not exist or is not "
>> +                         "active. Please, check the configuration in "
>> +                         "%s/template.conf to ensure it lists only 
>> valid "
>> +                         "networks." % (net_name, 
>> kimchiPaths.sysconf_dir))
>> +
>>               try:
>>                   net = conn.networkLookupByName(net_name)
>>               except libvirt.libvirtError, e:
>>                   msg = "Fatal: Unable to find network %s."
>>                   wok_log.error(msg, net_name)
>> -                wok_log.error(error_msg)
>>                   wok_log.error("Details: %s", e.message)
>> -                sys.exit(1)
>> +                raise Exception(error_msg)
>>
>>               if net.isActive() == 0:
>>                   try:
>> @@ -79,9 +78,8 @@ class NetworksModel(object):
>>                   except libvirt.libvirtError as e:
>>                       msg = "Fatal: Unable to activate network %s."
>>                       wok_log.error(msg, net_name)
>> -                    wok_log.error(error_msg)
>>                       wok_log.error("Details: %s", e.message)
>> -                    sys.exit(1)
>> +                    raise Exception(error_msg)
>>
>>       def create(self, params):
>>           conn = self.conn.get()
>> diff --git a/model/storagepools.py b/model/storagepools.py
>> index 3f6a734..a2dbaec 100644
>> --- a/model/storagepools.py
>> +++ b/model/storagepools.py
>> @@ -19,7 +19,6 @@
>>
>>   import libvirt
>>   import lxml.etree as ET
>> -import sys
>>   from lxml.builder import E
>>
>>   from wok.exception import InvalidOperation, MissingParameter
>> @@ -82,21 +81,21 @@ class StoragePoolsModel(object):
>>           if config.get('kimchi', {}).get('create_iso_pool', False):
>>               pools['ISO'] = {'path': '/var/lib/kimchi/isos'}
>>
>> -        error_msg = ("Please, check the configuration in 
>> %s/template.conf to "
>> -                     "ensure it has a valid storage pool." %
>> -                     kimchiPaths.sysconf_dir)
>> -
>>           conn = self.conn.get()
>>           for pool_name in pools:
>> +            error_msg = ("Storage pool %s does not exist or is not "
>> +                         "active. Please, check the configuration in "
>> +                         "%s/template.conf to ensure it lists only 
>> valid "
>> +                         "networks." % (pool_name, 
>> kimchiPaths.sysconf_dir))
>>               try:
>>                   pool = conn.storagePoolLookupByName(pool_name)
>>               except libvirt.libvirtError, e:
>>                   pool_path = pools[pool_name].get('path')
>>                   if pool_path is None:
>> -                    msg = "Fatal: Unable to find storage pool %s. " 
>> + error_msg
>> +                    msg = "Fatal: Unable to find storage pool %s. "
>>                       wok_log.error(msg % pool_name)
>>                       wok_log.error("Details: %s", e.message)
>> -                    sys.exit(1)
>> +                    raise Exception(error_msg)
>>
>>                   # Try to create the pool
>>                   pool = E.pool(E.name(pool_name), type='dir')
>> @@ -106,10 +105,9 @@ class StoragePoolsModel(object):
>>                       pool = conn.storagePoolDefineXML(xml, 0)
>>                   except libvirt.libvirtError, e:
>>                       msg = "Fatal: Unable to create storage pool %s. "
>> -                    msg += error_msg
>>                       wok_log.error(msg % pool_name)
>>                       wok_log.error("Details: %s", e.message)
>> -                    sys.exit(1)
>> +                    raise Exception(error_msg)
>>
>>                   # Build and set autostart value to pool
>>                   # Ignore error as the pool was already successfully 
>> created
>> @@ -127,10 +125,9 @@ class StoragePoolsModel(object):
>>                       pool.create(0)
>>                   except libvirt.libvirtError, e:
>>                       msg = "Fatal: Unable to craete storage pool %s. "
>> -                    msg += error_msg
>>                       wok_log.error(msg % pool_name)
>>                       wok_log.error("Details: %s", e.message)
>> -                    sys.exit(1)
>> +                    raise Exception(error_msg)
>>
>>       def get_list(self):
>>           try:
>