Adding additional exception handling and logging to WebUI.

3 files changed, 121 insertions, 61 deletions

diff --git a/cobbler/remote.py b/cobbler/remote.py
index cca0df9..2673cb9 100644
--- a/cobbler/remote.py
+++ b/cobbler/remote.py
@@ -146,7 +146,7 @@ class CobblerXMLRPCInterface:
         """
         return self.__get_all(self.api.repos())
 
-    def __get_specific(self,collection,name):
+    def __get_specific(self,collection,name,flatten=False):
         """
         Internal function to return a hash representation of a given object if it exists,
         otherwise an empty hash will be returned.
@@ -155,31 +155,34 @@ class CobblerXMLRPCInterface:
         item = collection.find(name=name)
         if item is None:
             return self._fix_none({})
-        return self._fix_none(item.to_datastruct())
+        result = item.to_datastruct()
+        if flatten:
+            result = utils.flatten(result)
+        return self._fix_none(result)
 
-    def get_distro(self,name,token=None):
+    def get_distro(self,name,flatten=False,token=None):
         """
         Returns the distro named "name" as a hash.
         """
-        return self.__get_specific(self.api.distros(),name)
+        return self.__get_specific(self.api.distros(),name,flatten=flatten)
 
-    def get_profile(self,name,token=None):
+    def get_profile(self,name,flatten=False,token=None):
         """
         Returns the profile named "name" as a hash.
         """
-        return self.__get_specific(self.api.profiles(),name)
+        return self.__get_specific(self.api.profiles(),name,flatten=flatten)
 
-    def get_system(self,name,token=None):
+    def get_system(self,name,flatten=False,token=None):
         """
         Returns the system named "name" as a hash.
         """
-        return self.__get_specific(self.api.systems(),name)
+        return self.__get_specific(self.api.systems(),name,flatten=flatten)
 
-    def get_repo(self,name,token=None):
+    def get_repo(self,name,flatten=False,token=None):
         """
         Returns the repo named "name" as a hash.
         """
-        return self.__get_specific(self.api.repos(),name)
+        return self.__get_specific(self.api.repos(),name,flatten=flatten)
 
     def get_distro_as_rendered(self,name,token=None):
         """
@@ -748,6 +751,8 @@ if __name__ == "__main__":
     print remote.get_systems()
     print remote.get_repos()
 
+    print remote.get_system("AA:BB:AA:BB:AA:BB",True) # flattened
+
     # now simulate hitting a "sync" button in a WebUI
     print remote.sync(token)
 
diff --git a/cobbler/utils.py b/cobbler/utils.py
index 23dcca6..6a7618d 100644
--- a/cobbler/utils.py
+++ b/cobbler/utils.py
@@ -22,6 +22,7 @@ import shutil
 import string
 import traceback
 import logging
+from cexceptions import *
 from rhpl.translate import _, N_, textdomain, utf8
 
 import api # factor out
@@ -231,7 +232,9 @@ def input_string_or_hash(options,delim=","):
     """
     if options is None:
         return (True, {})
-    elif type(options) != dict:
+    elif type(options) == list:
+        raise CX(_("No idea what to do with list: %s") % options)
+    elif type(options) == str:
         new_dict = {}
         tokens = options.split(delim)
         for t in tokens:
@@ -244,9 +247,11 @@ def input_string_or_hash(options,delim=","):
                 return (False, {})
         new_dict.pop('', None)
         return (True, new_dict)
-    else:
+    elif type(options) == dict:
         options.pop('',None)
         return (True, options)
+    else:
+        raise CX(_("Foreign options type"))
 
 def grab_tree(obj):
     """
@@ -288,11 +293,18 @@ def blender(remove_hashes, root_obj):
 
     # sanitize output for koan and kernel option lines, etc
     if remove_hashes:
-        results["kernel_options"] = hash_to_string(results["kernel_options"])
-        results["ks_meta"] = hash_to_string(results["ks_meta"])
+        results = flatten(results)
 
     return results
 
+def flatten(data):
+    # convert certain nested hashes to strings.
+    if data.has_key("kernel_options"):
+        data["kernel_options"] = hash_to_string(data["kernel_options"])
+    if data.has_key("ks_meta"):
+        data["ks_meta"] = hash_to_string(data["ks_meta"])
+    return data
+
 def __consolidate(node,results):
     """
     Merge data from a given node with the aggregate of all
diff --git a/cobbler/webui/CobblerWeb.py b/cobbler/webui/CobblerWeb.py
index 02fcde8..236ebcf 100644
--- a/cobbler/webui/CobblerWeb.py
+++ b/cobbler/webui/CobblerWeb.py
@@ -13,7 +13,27 @@ import exceptions
 import xmlrpclib
 from Cheetah.Template import Template
 import os
+import traceback
+import string
 from cobbler.utils import *
+import logging
+import sys
+
+logger = logging.getLogger("cobbler.webui")
+logger.setLevel(logging.DEBUG)
+ch = logging.FileHandler("/var/log/cobbler/webui.log")
+ch.setLevel(logging.DEBUG)
+formatter = logging.Formatter("%(asctime)s - %(name)s - %(levelname)s - %(message)s")
+ch.setFormatter(formatter)
+logger.addHandler(ch)
+
+def log_exc():
+   (t, v, tb) = sys.exc_info()
+   logger.info("Exception occured: %s" % t )
+   logger.info("Exception value: %s" % v)
+   logger.info("Exception Info:\n%s" % string.join(traceback.format_list(traceback.extract_tb(tb))))
+
+
 
 class CobblerWeb(object):
     """
@@ -34,7 +54,7 @@ class CobblerWeb(object):
         self.username = username
         self.password = password
 
-    def __xmlrpc(self):
+    def __xmlrpc_setup(self):
         """
         Sets up the connection to the Cobbler XMLRPC server.  Right now, the
         r/w server is required.   In the future, it may be possible to instantiate
@@ -51,11 +71,16 @@ class CobblerWeb(object):
         Call the templating engine (Cheetah), wrapping up the location
         of files while we're at it.
         """
-        data['base_url'] = self.base_url
-        #filepath = "%s/%s" % (os.path.dirname(__file__), template)
-        filepath = os.path.join("/usr/share/cobbler/webui_templates/",template)
-        tmpl = Template( file=filepath, searchList=data )
-        return str(tmpl)
+        try:
+            data['base_url'] = self.base_url
+            #filepath = "%s/%s" % (os.path.dirname(__file__), template)
+            filepath = os.path.join("/usr/share/cobbler/webui_templates/",template)
+            tmpl = Template( file=filepath, searchList=data )
+            return str(tmpl)
+        except:
+            logger.error("An error has occurred.") # FIXME: remove
+            log_exc()
+            return self.error_page("Error while rendering page.  See /var/log/cobbler/webui.log")
 
     def modes(self):
         """
@@ -80,8 +105,9 @@ class CobblerWeb(object):
     # Settings
     # ------------------------------------------------------------------------ #
     def settings_view(self):
+        self.__xmlrpc_setup()
         return self.__render( 'item.tmpl', {
-            'item_data': self.__xmlrpc().get_settings(),
+            'item_data': self.remote.get_settings(),
             'caption':   "Cobbler Settings"
         } )
 
@@ -89,16 +115,16 @@ class CobblerWeb(object):
     # Distributions
     # ------------------------------------------------------------------------ #
     def distro_view(self, distribution):
-        # get_distro_for_koan() flattens out the inherited options
-        #distro = self.__xmlrpc().get_distro_for_koan(distribution)
+        self.__xmlrpc_setup()
         return  self.__render( 'item.tmpl', {
-            'item_data': self.__xmlrpc().get_distro(distribution),
+            'item_data': self.remote.get_distro(distribution,True),
             'caption':   "Distribution \"%s\" Details" % distribution
         } )
 
     def distro_list(self):
+        self.__xmlrpc_setup()
         return self.__render( 'distro_list.tmpl', {
-            'distros': self.__xmlrpc().get_distros()
+            'distros': self.remote.get_distros()
         } )
 
     # ------------------------------------------------------------------------ #
@@ -107,83 +133,99 @@ class CobblerWeb(object):
     # if the system list is huge, this will probably need to use an
     # iterator so the list doesn't get copied around
     def system_list(self):
+        self.__xmlrpc_setup()
         return self.__render( 'system_list.tmpl', {
-            'systems': self.__xmlrpc().get_systems()
+            'systems': self.remote.get_systems()
         } )
 
     def system_add(self):
+        self.__xmlrpc_setup()
         return self.__render( 'system_edit.tmpl', {
             'system': None,
-            'profiles': self.__xmlrpc().get_profiles()
+            'profiles': self.remote.get_profiles()
         } )
 
     def system_view(self, name):
+        self.__xmlrpc_setup()
         return self.__render( 'item.tmpl', {
-            'item_data': self.__xmlrpc().get_system(name),
+            'item_data': self.remote.get_system(name,True),
             'caption':   "Profile %s Settings" % name
         } )
 
-    def system_save(self, name=None, profile=None, new_or_edit=None, mac=None, ip=None, hostname=None, kopts=None, ksmeta=None, netboot='n', **args):
+    def system_save(self, name=None, profile=None, new_or_edit=None, mac=None, ip=None, hostname=None, kopts=None, ksmeta=None, netboot='n', dhcp_tag=None, **args):
+        self.__xmlrpc_setup()
+
         # parameter checking
         if name is None:
             return self.error_page("System name parameter is REQUIRED.")
-
-        if mac is None and ip is None and hostname is None:
+        if mac is None and ip is None and hostname is None and not is_mac(name):
             return self.error_page("System must have at least one of MAC/IP/hostname.")
-
-        # resolve_ip, is_mac, and is_ip are from cobbler.utils
         if hostname and not ip:
             ip = resolve_ip( hostname )
-
         if mac and not is_mac( mac ):
             return self.error_page("The provided MAC address appears to be invalid.")
-
         if ip and not is_ip( ip ):
             return self.error_page("The provided IP address appears to be invalid.")
 
-        # set up XMLRPC - token is in self.token
-        self.__xmlrpc()
-
+        # grab a reference to the object
         if new_or_edit == "edit":
-            system = self.remote.get_system_handle( name, self.token )
+            try:
+                system = self.remote.get_system_handle( name, self.token )
+            except:
+                return self.error_page("Failed to lookup system: %s" % name)
         else:
             system = self.remote.new_system( self.token )
-            self.remote.modify_system( system, 'name', name, self.token )
-
-        if profile:
-            self.remote.modify_system(system, 'profile',  profile,  self.token)
-        if mac:
-            self.remote.modify_system(system, 'mac',      mac,      self.token)
-        if ip:
-            self.remote.modify_system(system, 'ip',       ip,       self.token)
-        if hostname:
-            self.remote.modify_system(system, 'hostname', hostname, self.token)
-        if kopts:
-            self.remote.modify_system(system, 'kopts',    kopts,    self.token)
-        if ksmeta:
-            self.remote.modify_system(system, 'ksmeta',   ksmeta,   self.token)
-
-        self.remote.save_system( system, self.token )
 
+        # go!
+        try:
+            self.remote.modify_system(system, 'name', name, self.token )
+            self.remote.modify_system(system, 'profile', profile, self.token)
+            if mac:
+               self.remote.modify_system(system, 'mac', mac, self.token)
+            if ip:
+               self.remote.modify_system(system, 'ip', ip, self.token)
+            if hostname:
+               self.remote.modify_system(system, 'hostname', hostname, self.token)
+            if kopts:
+               self.remote.modify_system(system, 'kopts', kopts, self.token)
+            if ksmeta:
+               self.remote.modify_system(system, 'ksmeta', ksmeta, self.token)
+            if netboot:
+               self.remote.modify_system(system, 'netboot-enabled', netboot, self.token)
+            if dhcp_tag:
+               self.remote.modify_system(system, 'dhcp-tag', dhcp_tag, self.token)
+            self.remote.save_system( system, self.token)
+        except Exception, e:
+            # FIXME: get the exact error message and display to the user.
+            log_exc()
+            return self.error_page("Error while saving system: %s" % str(e))
         return self.system_view( name=name )
 
+
+    #def tb2str(self,tb):
+    #    print " ".join(traceback.format_list(traceback.extract_tb(tb)))
+    #    return ""
+
     def system_edit(self, name):
+        self.__xmlrpc_setup()
         return self.__render( 'system_edit.tmpl', {
-            'system': self.__xmlrpc().get_system(name),
-            'profiles': self.__xmlrpc().get_profiles()
+            'system': self.remote.get_system(name,True),
+            'profiles': self.remote.get_profiles()
         } )
 
     # ------------------------------------------------------------------------ #
     # Profiles
     # ------------------------------------------------------------------------ #
     def profile_list(self):
+        self.__xmlrpc_setup()
         return self.__render( 'profile_list.tmpl', {
-            'profiles': self.__xmlrpc().get_profiles()
+            'profiles': self.remote.get_profiles()
         } )
 
     def profile_add(self):
+        self.__xmlrpc_setup()
         return self.__render( 'profile_add.tmpl', {
-            'distros': self.__xmlrpc().get_distros(),
+            'distros': self.remote.get_distros(),
             'ksfiles': self.__ksfiles()
         } )
 
@@ -205,8 +247,9 @@ class CobblerWeb(object):
         } )
 
     def __ksfiles(self):
-        ksfiles = list()
-        for profile in self.__xmlrpc().get_profiles():
+        self.__xmlrpc_setup()
+        ksfiles = []
+        for profile in self.remote.get_profiles():
             ksfile = profile['kickstart']
             if not ksfile in ksfiles:
                 ksfiles.append( ksfile )