On 05/29/2012 12:46 PM, Martin Kosek wrote:
On Tue, 2012-05-22 at 15:45 +0200, Petr Viktorin wrote:
On 2012-04-23 17:05, John Dennis wrote:
On 04/23/2012 05:19 AM, Petr Viktorin wrote:
This fixes https://fedorahosted.org/freeipa/ticket/2071 (Add final debug
message in installers).

I submitted an earlier version of this patch before (0014), but it was
too much to include in 2.2. Hopefully now there's more space for
restructuring. I think it's better to start a new thread with this
approach.

The try/except blocks at the end of installers/management scripts are
replaced by a call to a common function, which includes the final
message.
For each specific error, the error handlers in all scripts was almost
the same, but each script handled a different selection of errors.
Instead of having this copy/pasted code (with subtle differences
creeping in over time), this patch consolidates it all in one place.

I like this approach much better than the earlier patch, great, thanks.
I'm a big fan of calling into common code instead of copying code to my
mind the refactoring to utilize common code is great approach. I also
like the fact the logging configuration is not modified after it's
established.

At some point we may want to revist how the log messages are generated.
For example should all communication to the console pass through the
console handler? Is there a logger established for the script? Should
the format of messages emitted to the console be altered? Should all
command line utilities accept the both the verbose and debug flag? Etc.
But for now this is fantastic start in the right direction.

I have not installed and exercised the patch so I can't comment on any
runtime time issues that might be present, but from code inspection only
it has my ACK.


Thanks John!
Yes, this is just a start.


Patch rebased to curent master


The patch needs another rebase.

Besides that, the approach is fine (and removes a lot of redundant code)
but I found several issues with the patches:

Thanks for the review!

I was just looking into the issue. It missed that when logging is set up via api.bootstrap, by default the same logging level is set in both the log and console handlers. This makes it impossible to log only to the file -- INFO messages go to both file and console, and lower-level ones don't appear at al. Fixing this will require changes to the logging setup of individual commands, and possibly the logging mechanism itself, which is more than I want to include in this patch.

Most scripts where this happens are not top-level installers (they're ipa-compliance, ipa-replica-conncheck, ipa-replica-manage, ipa-replica-prepare, ipa-ldap-updater). Since the ticket only asks for installers, I reverted the changes in these. Fixing them is left to ticket #2652.
So, issues 2-3 & 5-10 that you found are avoided.
I'll keep the cases around to test further work.


I changed ipa-server-certinstall to set up logging explicitly.


1) ipa-server-install: Fails when option parser error is raised:

# ipa-server-install -p foo
Usage: ipa-server-install [options]

ipa-server-install: error: DS admin password: Password must be at least
8 characters long
Traceback (most recent call last):
   File "/sbin/ipa-server-install", line 1131, in<module>
     if not success and installation_cleanup:
NameError: name 'success' is not defined

Fixed

4) ipa-ca-install emits failed_message when option error is detected:

# ipa-ca-install
Usage: ipa-ca-install [options] REPLICA_FILE

ipa-ca-install: error: you must provide a file generated by
ipa-replica-prepare

Your system may be partly configured.
Run /usr/sbin/ipa-server-install --uninstall to clean up.

The attached patch skips printing the message on SystemExit, as it's done without the patch.


[ipa-csreplica-manage] also has a wrong operation_name. This is what
happens when a
script name is hard-coded and it is not detected automatically, e.g.
from __file__.

I'll keep that in mind for #2652. Perhaps I'm trying too hard to avoid magic.

Martin



--
PetrĀ³
From 600674c03a1fbd97a7d3c2f4c9471e779380d257 Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pvikt...@redhat.com>
Date: Fri, 20 Apr 2012 04:39:59 -0400
Subject: [PATCH] Move install script error handling to a common function

All of our install/admin scripts had a try/except block calling the
main function and handling common exceptions. These were copy-pasted
from each other and modified to various levels of sophistication.
This refactors them out of installers to a single function, which
includes a final pass/fail message for all of the scripts.

Non-install scripts that set up the same log handler levels for
stderr and log file are not changed, as it's not possible to log
to only the logfile without changing the logger configuration.

https://fedorahosted.org/freeipa/ticket/2071
---
 install/tools/ipa-adtrust-install    |   34 ++-------
 install/tools/ipa-ca-install         |   65 +++++++----------
 install/tools/ipa-compat-manage      |   23 +-----
 install/tools/ipa-dns-install        |   34 ++-------
 install/tools/ipa-managed-entries    |   19 +----
 install/tools/ipa-nis-manage         |   23 +-----
 install/tools/ipa-replica-install    |   64 +++++++----------
 install/tools/ipa-server-certinstall |   17 +++--
 install/tools/ipa-server-install     |   57 +++++++--------
 install/tools/ipa-upgradeconfig      |    9 +--
 install/tools/ipactl                 |   30 ++------
 ipaserver/install/installutils.py    |  130 +++++++++++++++++++++++++++++++++-
 ipaserver/install/ldapupdate.py      |    5 +-
 13 files changed, 243 insertions(+), 267 deletions(-)

diff --git a/install/tools/ipa-adtrust-install b/install/tools/ipa-adtrust-install
index 248ea35eaa86dd59ebbc871b86df780cfd71ccf6..0dfc6eba6568f2388fb2bec8319acf593ad838a9 100755
--- a/install/tools/ipa-adtrust-install
+++ b/install/tools/ipa-adtrust-install
@@ -21,8 +21,6 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 
-import traceback
-
 from ipaserver.plugins.ldap2 import ldap2
 from ipaserver.install import adtrustinstance
 from ipaserver.install.installutils import *
@@ -35,6 +33,8 @@ import krbV
 import ldap
 from ipapython.ipa_log_manager import *
 
+log_file_name = "/var/log/ipaserver-install.log"
+
 def parse_options():
     parser = IPAOptionParser(version=version.VERSION)
     parser.add_option("-p", "--ds-password", dest="dm_password",
@@ -86,8 +86,8 @@ def main():
     if os.getegid() != 0:
         sys.exit("Must be root to setup AD trusts on server")
 
-    standard_logging_setup("/var/log/ipaserver-install.log", debug=options.debug, filemode='a')
-    print "\nThe log file for this installation can be found in /var/log/ipaserver-install.log"
+    standard_logging_setup(log_file_name, debug=options.debug, filemode='a')
+    print "\nThe log file for this installation can be found in %s" % log_file_name
 
     root_logger.debug('%s was invoked with options: %s' % (sys.argv[0], safe_options))
     root_logger.debug("missing options might be asked for interactively later\n")
@@ -227,26 +227,6 @@ def main():
 
     return 0
 
-try:
-    sys.exit(main())
-except SystemExit, e:
-    sys.exit(e)
-except KeyboardInterrupt:
-    print "Installation cancelled."
-except RuntimeError, e:
-    print str(e)
-except HostnameLocalhost:
-    print "The hostname resolves to the localhost address (127.0.0.1/::1)"
-    print "Please change your /etc/hosts file so that the hostname"
-    print "resolves to the ip address of your network interface."
-    print "The KDC service does not listen on localhost"
-    print ""
-    print "Please fix your /etc/hosts file and restart the setup program"
-except Exception, e:
-    message = "Unexpected error - see ipaserver-install.log for details:\n %s" % str(e)
-    print message
-    message = str(e)
-    for str in traceback.format_tb(sys.exc_info()[2]):
-        message = message + "\n" + str
-    root_logger.debug(message)
-    sys.exit(1)
+if __name__ == '__main__':
+    installutils.run_script(main, log_file_name=log_file_name,
+            operation_name='ipa-adtrust-install')
diff --git a/install/tools/ipa-ca-install b/install/tools/ipa-ca-install
index 55b4dbfb89cf0591a2c4d3ce827a8bb06bb3a9f3..4d7be217d2d19cb769990a58d4ff78b6b3dc698e 100755
--- a/install/tools/ipa-ca-install
+++ b/install/tools/ipa-ca-install
@@ -21,7 +21,7 @@
 import sys
 import socket
 
-import os, traceback, shutil
+import os, shutil
 
 from ipapython import ipautil
 from ipapython import services as ipaservices
@@ -39,8 +39,9 @@ from ipapython.config import IPAOptionParser
 from ipapython import sysrestore
 from ipapython.ipa_log_manager import *
 
-CACERT="/etc/ipa/ca.crt"
-REPLICA_INFO_TOP_DIR=None
+log_file_name = "/var/log/ipareplica-ca-install.log"
+CACERT = "/etc/ipa/ca.crt"
+REPLICA_INFO_TOP_DIR = None
 
 def parse_options():
     usage = "%prog [options] REPLICA_FILE"
@@ -72,7 +73,12 @@ def get_dirman_password():
 
 def main():
     safe_options, options, filename = parse_options()
-    standard_logging_setup("/var/log/ipareplica-ca-install.log", debug=options.debug)
+
+    if os.geteuid() != 0:
+        sys.exit("\nYou must be root to run this script.\n")
+
+    standard_logging_setup(log_file_name, debug=options.debug)
+
     root_logger.debug('%s was invoked with argument "%s" and options: %s' % (sys.argv[0], filename, safe_options))
 
     if not ipautil.file_exists(filename):
@@ -150,41 +156,20 @@ def main():
     # We need to restart apache as we drop a new config file in there
     ipaservices.knownservices.httpd.restart(capture_output=True)
 
-try:
-    if not os.geteuid()==0:
-        sys.exit("\nYou must be root to run this script.\n")
+fail_message = '''
+Your system may be partly configured.
+Run /usr/sbin/ipa-server-install --uninstall to clean up.
+'''
 
-    main()
-    sys.exit(0)
-except SystemExit, e:
-    sys.exit(e)
-except socket.error, (errno, errstr):
-    print errstr
-except HostnameLocalhost:
-    print "The hostname resolves to the localhost address (127.0.0.1/::1)"
-    print "Please change your /etc/hosts file so that the hostname"
-    print "resolves to the ip address of your network interface."
-    print ""
-    print "Please fix your /etc/hosts file and restart the setup program"
-except Exception, e:
-    print "creation of replica failed: %s" % str(e)
-    message = str(e)
-    for str in traceback.format_tb(sys.exc_info()[2]):
-        message = message + "\n" + str
-    root_logger.debug(message)
-except KeyboardInterrupt:
-    print "Installation cancelled."
-finally:
-    # always try to remove decrypted replica file
+if __name__ == '__main__':
     try:
-        if REPLICA_INFO_TOP_DIR:
-            shutil.rmtree(REPLICA_INFO_TOP_DIR)
-    except OSError:
-        pass
-
-print ""
-print "Your system may be partly configured."
-print "Run /usr/sbin/ipa-server-install --uninstall to clean up."
-
-# the only way to get here is on error or ^C
-sys.exit(1)
+        installutils.run_script(main, log_file_name=log_file_name,
+                operation_name='ipa-ca-install',
+                fail_message=fail_message)
+    finally:
+        # always try to remove decrypted replica file
+        try:
+            if REPLICA_INFO_TOP_DIR:
+                shutil.rmtree(REPLICA_INFO_TOP_DIR)
+        except OSError:
+            pass
diff --git a/install/tools/ipa-compat-manage b/install/tools/ipa-compat-manage
index 13a93cbed02ca69f4f6e8cb156a2f6f18e2da899..f7564e0c59fd2b310bc15de1ac0ffdb80217fe9b 100755
--- a/install/tools/ipa-compat-manage
+++ b/install/tools/ipa-compat-manage
@@ -196,24 +196,5 @@ def main():
 
     return retval
 
-try:
-    if __name__ == "__main__":
-        sys.exit(main())
-except BadSyntax, e:
-    print "There is a syntax error in this update file:"
-    print "  %s" % e
-    sys.exit(1)
-except RuntimeError, e:
-    print "%s" % e
-    sys.exit(1)
-except SystemExit, e:
-    sys.exit(e)
-except KeyboardInterrupt, e:
-    sys.exit(1)
-except config.IPAConfigError, e:
-    print "An IPA server to update cannot be found. Has one been configured yet?"
-    print "The error was: %s" % e
-    sys.exit(1)
-except errors.LDAPError, e:
-    print "An error occurred while performing operations: %s" % e
-    sys.exit(1)
+if __name__ == '__main__':
+    installutils.run_script(main, operation_name='ipa-compat-manage')
diff --git a/install/tools/ipa-dns-install b/install/tools/ipa-dns-install
index 10547c3425c0ac3d356665d47c5407e2180b1fab..063cf5bf5747067bfab60d5d1e3c69af5ef72f83 100755
--- a/install/tools/ipa-dns-install
+++ b/install/tools/ipa-dns-install
@@ -19,8 +19,6 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 
-import traceback
-
 from ipaserver.plugins.ldap2 import ldap2
 from ipaserver.install import service, bindinstance, ntpinstance, httpinstance
 from ipaserver.install.installutils import *
@@ -34,6 +32,8 @@ import krbV
 import ldap
 from ipapython.ipa_log_manager import *
 
+log_file_name = "/var/log/ipaserver-install.log"
+
 def parse_options():
     parser = IPAOptionParser(version=version.VERSION)
     parser.add_option("-p", "--ds-password", dest="dm_password",
@@ -89,8 +89,8 @@ def main():
     if os.getegid() != 0:
         sys.exit("Must be root to setup server")
 
-    standard_logging_setup("/var/log/ipaserver-install.log", debug=options.debug, filemode='a')
-    print "\nThe log file for this installation can be found in /var/log/ipaserver-install.log"
+    standard_logging_setup(log_file_name, debug=options.debug, filemode='a')
+    print "\nThe log file for this installation can be found in %s" % log_file_name
 
     root_logger.debug('%s was invoked with options: %s' % (sys.argv[0], safe_options))
     root_logger.debug("missing options might be asked for interactively later\n")
@@ -243,26 +243,6 @@ def main():
 
     return 0
 
-try:
-    sys.exit(main())
-except SystemExit, e:
-    sys.exit(e)
-except KeyboardInterrupt:
-    print "Installation cancelled."
-except RuntimeError, e:
-    print str(e)
-except HostnameLocalhost:
-    print "The hostname resolves to the localhost address (127.0.0.1/::1)"
-    print "Please change your /etc/hosts file so that the hostname"
-    print "resolves to the ip address of your network interface."
-    print "The KDC service does not listen on localhost"
-    print ""
-    print "Please fix your /etc/hosts file and restart the setup program"
-except Exception, e:
-    message = "Unexpected error - see ipaserver-install.log for details:\n %s" % str(e)
-    print message
-    message = str(e)
-    for str in traceback.format_tb(sys.exc_info()[2]):
-        message = message + "\n" + str
-    root_logger.debug(message)
-    sys.exit(1)
+if __name__ == '__main__':
+    installutils.run_script(main, log_file_name=log_file_name,
+        operation_name='ipa-dns-install')
diff --git a/install/tools/ipa-managed-entries b/install/tools/ipa-managed-entries
index 00bb566226adf636fe1c2cfc4a6357636f3ffb71..b9a492e48d87ca72f7c0043c8a4a5a26b77be847 100755
--- a/install/tools/ipa-managed-entries
+++ b/install/tools/ipa-managed-entries
@@ -237,20 +237,5 @@ def main():
 
     return retval
 
-try:
-    if __name__ == "__main__":
-        sys.exit(main())
-except RuntimeError, e:
-    print "%s" % e
-    sys.exit(1)
-except SystemExit, e:
-    sys.exit(e)
-except KeyboardInterrupt, e:
-    sys.exit(1)
-except config.IPAConfigError, e:
-    print "An IPA server to update cannot be found. Has one been configured yet?"
-    print "The error was: %s" % e
-    sys.exit(1)
-except errors.LDAPError, e:
-    print "An error occurred while performing operations: %s" % e
-    sys.exit(1)
+if __name__ == '__main__':
+    installutils.run_script(main, operation_name='ipa-managed-entries')
diff --git a/install/tools/ipa-nis-manage b/install/tools/ipa-nis-manage
index 5c5bbca8e0435441cbb2ea10d80245e36a86e9a7..1c6de7b57d61bc1ecb8f69833a4b76a9af45a618 100755
--- a/install/tools/ipa-nis-manage
+++ b/install/tools/ipa-nis-manage
@@ -200,24 +200,5 @@ def main():
 
     return retval
 
-try:
-    if __name__ == "__main__":
-        sys.exit(main())
-except BadSyntax, e:
-    print "There is a syntax error in this update file:"
-    print "  %s" % e
-    sys.exit(1)
-except RuntimeError, e:
-    print "%s" % e
-    sys.exit(1)
-except SystemExit, e:
-    sys.exit(e)
-except KeyboardInterrupt, e:
-    sys.exit(1)
-except config.IPAConfigError, e:
-    print "An IPA server to update cannot be found. Has one been configured yet?"
-    print "The error was: %s" % e
-    sys.exit(1)
-except errors.LDAPError, e:
-    print "An error occurred while performing operations: %s" % e
-    sys.exit(1)
+if __name__ == '__main__':
+    installutils.run_script(main, operation_name='ipa-nis-manage')
diff --git a/install/tools/ipa-replica-install b/install/tools/ipa-replica-install
index 7cfe7627e2d0b85d3584ab757c3ca5b78c51b801..c5292f3fc1aa94ec11688176f6167a964c3ee11b 100755
--- a/install/tools/ipa-replica-install
+++ b/install/tools/ipa-replica-install
@@ -21,7 +21,7 @@
 import sys
 import socket
 
-import os, pwd, traceback, shutil
+import os, pwd, shutil
 import grp
 from optparse import OptionGroup
 
@@ -43,8 +43,9 @@ from ipapython import sysrestore
 from ipapython import services as ipaservices
 from ipapython.ipa_log_manager import *
 
-CACERT="/etc/ipa/ca.crt"
-REPLICA_INFO_TOP_DIR=None
+log_file_name = "/var/log/ipareplica-install.log"
+CACERT = "/etc/ipa/ca.crt"
+REPLICA_INFO_TOP_DIR = None
 
 def parse_options():
     usage = "%prog [options] REPLICA_FILE"
@@ -277,7 +278,11 @@ def check_bind():
 
 def main():
     safe_options, options, filename = parse_options()
-    standard_logging_setup("/var/log/ipareplica-install.log", debug=options.debug)
+
+    if os.geteuid() != 0:
+        sys.exit("\nYou must be root to run this script.\n")
+
+    standard_logging_setup(log_file_name, debug=options.debug)
     root_logger.debug('%s was invoked with argument "%s" and options: %s' % (sys.argv[0], filename, safe_options))
 
     if not ipautil.file_exists(filename):
@@ -501,41 +506,20 @@ def main():
     #Everything installed properly, activate ipa service.
     ipaservices.knownservices.ipa.enable()
 
-try:
-    if not os.geteuid()==0:
-        sys.exit("\nYou must be root to run this script.\n")
+fail_message = '''
+Your system may be partly configured.
+Run /usr/sbin/ipa-server-install --uninstall to clean up.
+'''
 
-    main()
-    sys.exit(0)
-except SystemExit, e:
-    sys.exit(e)
-except socket.error, (errno, errstr):
-    print errstr
-except HostnameLocalhost:
-    print "The hostname resolves to the localhost address (127.0.0.1/::1)"
-    print "Please change your /etc/hosts file so that the hostname"
-    print "resolves to the ip address of your network interface."
-    print ""
-    print "Please fix your /etc/hosts file and restart the setup program"
-except Exception, e:
-    print "creation of replica failed: %s" % str(e)
-    message = str(e)
-    for str in traceback.format_tb(sys.exc_info()[2]):
-        message = message + "\n" + str
-    root_logger.debug(message)
-except KeyboardInterrupt:
-    print "Installation cancelled."
-finally:
-    # always try to remove decrypted replica file
+if __name__ == '__main__':
     try:
-        if REPLICA_INFO_TOP_DIR:
-            shutil.rmtree(REPLICA_INFO_TOP_DIR)
-    except OSError:
-        pass
-
-print ""
-print "Your system may be partly configured."
-print "Run /usr/sbin/ipa-server-install --uninstall to clean up."
-
-# the only way to get here is on error or ^C
-sys.exit(1)
+        installutils.run_script(main, log_file_name=log_file_name,
+                operation_name='ipa-replica-install',
+                fail_message=fail_message)
+    finally:
+        # always try to remove decrypted replica file
+        try:
+            if REPLICA_INFO_TOP_DIR:
+                shutil.rmtree(REPLICA_INFO_TOP_DIR)
+        except OSError:
+            pass
diff --git a/install/tools/ipa-server-certinstall b/install/tools/ipa-server-certinstall
index 901678b2e8e250ed7d460df326ff965e74d6167e..3b19f0452a040d5e3753a1e9bab684439a0ab05d 100755
--- a/install/tools/ipa-server-certinstall
+++ b/install/tools/ipa-server-certinstall
@@ -31,6 +31,7 @@ from ipapython.ipautil import user_input
 
 from ipaserver.install import certs, dsinstance, httpinstance, installutils
 from ipalib import api
+from ipapython.ipa_log_manager import *
 from ipaserver.plugins.ldap2 import ldap2
 
 def get_realm_name():
@@ -120,12 +121,17 @@ def import_cert(dirname, pkcs12_fname, pkcs12_passwd, db_password):
     return server_cert
 
 def main():
+    if os.geteuid() != 0:
+        sys.exit("\nYou must be root to run this script.\n")
+
     installutils.check_server_configuration()
 
     options, pkcs12_fname = parse_options()
 
     cfg = dict(in_server=True,)
 
+    standard_logging_setup("/var/log/ipa/default.log")
+
     api.bootstrap(**cfg)
     api.finalize()
 
@@ -165,12 +171,5 @@ def main():
 
     return 0
 
-try:
-    if not os.geteuid()==0:
-        sys.exit("\nYou must be root to run this script.\n")
-
-    main()
-except SystemExit, e:
-    sys.exit(e)
-except RuntimeError, e:
-    sys.exit(e)
+if __name__ == '__main__':
+    installutils.run_script(main, operation_name='ipa-server-certinstall')
diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install
index 2f06a9e879902eb1c2ac340757fcd1762959fe30..dfdc5cb94d7d78b7e1542fc430f60e6c039996b1 100755
--- a/install/tools/ipa-server-install
+++ b/install/tools/ipa-server-install
@@ -34,7 +34,6 @@ import subprocess
 import signal
 import shutil
 import glob
-import traceback
 import pickle
 import random
 import tempfile
@@ -50,7 +49,7 @@ from ipaserver.install import certs
 from ipaserver.install import cainstance
 from ipaserver.install import memcacheinstance
 
-from ipaserver.install import service
+from ipaserver.install import service, installutils
 from ipapython import version
 from ipaserver.install.installutils import *
 from ipaserver.plugins.ldap2 import ldap2
@@ -1108,37 +1107,29 @@ def main():
         os.remove(ANSWER_CACHE)
     return 0
 
-try:
-    success = True
+if __name__ == '__main__':
+    success = False
     try:
-        rval = main()
-        if rval != 0:
-            success = False
-        sys.exit(rval)
-    except SystemExit, e:
-        if e.code is not None or e.code != 0:
-            success = False
-        sys.exit(e)
-    except Exception, e:
-        success = False
-        if uninstalling:
-            message = "Unexpected error - see ipaserver-uninstall.log for details:\n %s" % unicode(e)
+        # FIXME: Common option parsing, logging setup, etc should be factored
+        # out from all install scripts
+        safe_options, options = parse_options()
+        if options.uninstall:
+            log_file_name = "/var/log/ipaserver-uninstall.log"
         else:
-            message = "Unexpected error - see ipaserver-install.log for details:\n %s" % unicode(e)
-        print message
-        message = str(e)
-        for str in traceback.format_tb(sys.exc_info()[2]):
-            message = message + "\n" + str
-        root_logger.debug(message)
-        sys.exit(1)
-finally:
-    if pw_name and ipautil.file_exists(pw_name):
-        os.remove(pw_name)
+            log_file_name = "/var/log/ipaserver-install.log"
 
-    if not success and installation_cleanup:
-        # Do a cautious clean up as we don't know what failed and what is
-        # the state of the environment
-        try:
-            fstore.restore_file('/etc/hosts')
-        except:
-            pass
+        installutils.run_script(main, log_file_name=log_file_name,
+            operation_name='ipa-server-install')
+        success = True
+
+    finally:
+        if pw_name and ipautil.file_exists(pw_name):
+            os.remove(pw_name)
+
+        if not success and installation_cleanup:
+            # Do a cautious clean up as we don't know what failed and what is
+            # the state of the environment
+            try:
+                fstore.restore_file('/etc/hosts')
+            except:
+                pass
diff --git a/install/tools/ipa-upgradeconfig b/install/tools/ipa-upgradeconfig
index a2a30249923ed127d2d68d312ad7abeb04627678..0cf59f293b6a35a7b4fddda14a729413ab51e17f 100644
--- a/install/tools/ipa-upgradeconfig
+++ b/install/tools/ipa-upgradeconfig
@@ -305,10 +305,5 @@ def main():
     cleanup_kdc(fstore)
     upgrade_ipa_profile(krbctx.default_realm)
 
-try:
-    if __name__ == "__main__":
-        sys.exit(main())
-except SystemExit, e:
-    sys.exit(e)
-except KeyboardInterrupt, e:
-    sys.exit(1)
+if __name__ == '__main__':
+    installutils.run_script(main, operation_name='ipa-upgradeconfig')
diff --git a/install/tools/ipactl b/install/tools/ipactl
index 74ee383047618ac3053d61d40a8eb13e27f7fc78..c4d26b8df150119e0bc84abac020f8989a2a8ad2 100755
--- a/install/tools/ipactl
+++ b/install/tools/ipactl
@@ -21,10 +21,10 @@
 import sys
 try:
     import os
-    from ipaserver.install import service
+    from ipaserver.install import service, installutils
     from ipapython import services as ipaservices
     from ipaserver.install.dsinstance import config_dirname, realm_to_serverid
-    from ipaserver.install.installutils import is_ipa_configured, wait_for_open_ports, wait_for_open_socket
+    from ipaserver.install.installutils import is_ipa_configured, wait_for_open_ports, wait_for_open_socket, ScriptError
     from ipapython import sysrestore
     from ipapython import config
     from ipalib import api, errors
@@ -44,13 +44,8 @@ error was:
 
 SASL_EXTERNAL = ldap.sasl.sasl({}, 'EXTERNAL')
 
-class IpactlError(StandardError):
-    def __init__(self, msg = '', rval = 1):
-        self.msg = msg
-        self.rval = rval
-
-    def __str__(self):
-        return self.msg
+class IpactlError(ScriptError):
+    pass
 
 def check_IPA_configuration():
     if not is_ipa_configured():
@@ -386,17 +381,6 @@ def main():
     elif args[0].lower() == "status":
         ipa_status(options)
 
-try:
-    if __name__ == "__main__":
-        sys.exit(main())
-except IpactlError, e:
-    if e.msg:
-        emit_err(e.msg)
-    sys.exit(e.rval)
-except RuntimeError, e:
-    emit_err("%s" % e)
-    sys.exit(1)
-except SystemExit, e:
-    sys.exit(e)
-except KeyboardInterrupt, e:
-    sys.exit(1)
+
+if __name__ == '__main__':
+    installutils.run_script(main, operation_name='ipactl')
diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py
index 362723614749449b15087653e6c4f180f98d800d..313761777cddcbf5ad9dd134556d72931e51a2b3 100644
--- a/ipaserver/install/installutils.py
+++ b/ipaserver/install/installutils.py
@@ -30,13 +30,18 @@
 import time
 import tempfile
 import shutil
+from ConfigParser import SafeConfigParser
+import traceback
+
 from dns import resolver, rdatatype
 from dns.exception import DNSException
+import ldap
 
-from ConfigParser import SafeConfigParser
 from ipapython import ipautil, sysrestore
 from ipapython.ipa_log_manager import *
 from ipalib.util import validate_hostname
+from ipapython import config
+from ipalib import errors
 
 # Used to determine install status
 IPA_MODULES = ['httpd', 'kadmin', 'dirsrv', 'pki-cad', 'pkids', 'install', 'krb5kdc', 'ntpd', 'named', 'ipa_memcached']
@@ -56,6 +61,18 @@ class HostReverseLookupError(HostLookupError):
 class HostnameLocalhost(HostLookupError):
     pass
 
+
+class ScriptError(StandardError):
+    """An exception that records an error message and a return value
+    """
+    def __init__(self, msg = '', rval = 1):
+        self.msg = msg
+        self.rval = rval
+
+    def __str__(self):
+        return self.msg
+
+
 class ReplicaConfig:
     def __init__(self):
         self.realm_name = ""
@@ -655,3 +672,114 @@ def is_ipa_configured():
         root_logger.debug('filestore is tracking no files')
 
     return installed
+
+
+def run_script(main_function, operation_name, log_file_name=None,
+        fail_message=None):
+    """Run the given function as a command-line utility
+
+    This function:
+
+    - Runs the given function
+    - Formats any errors
+    - Exits with the appropriate code
+
+    :param main_function: Function to call
+    :param log_file_name: Name of the log file (displayed on unexpected errors)
+    :param operation_name: Name of the script
+    :param fail_message: Optional message displayed on failure
+    """
+
+    root_logger.info('Starting script: %s', operation_name)
+    try:
+        try:
+            return_value = main_function()
+        except BaseException, e:
+            if isinstance(e, SystemExit) and (e.code is None or e.code == 0):
+                # Not an error after all
+                root_logger.info('The %s command was successful',
+                    operation_name)
+            else:
+                # Log at the INFO level, which is not output to the console
+                # (unless in debug/verbose mode), but is written to a logfile
+                # if one is open.
+                tb = sys.exc_info()[2]
+                root_logger.info('\n'.join(traceback.format_tb(tb)))
+                root_logger.info('The %s command failed, exception: %s: %s',
+                    operation_name, type(e).__name__, e)
+                exception = e
+                if fail_message and not isinstance(e, SystemExit):
+                    print fail_message
+                raise
+        else:
+            if return_value:
+                root_logger.info('The %s command failed, return value %s',
+                    operation_name, return_value)
+            else:
+                root_logger.info('The %s command was successful',
+                    operation_name)
+            sys.exit(return_value)
+
+    except BaseException, error:
+        handle_error(error, log_file_name)
+
+
+def handle_error(error, log_file_name=None):
+    """Handle specific errors"""
+
+    if isinstance(error, SystemExit):
+        sys.exit(error)
+    if isinstance(error, RuntimeError):
+        sys.exit(error)
+    if isinstance(error, KeyboardInterrupt):
+        print >> sys.stderr, "Cancelled."
+        sys.exit(1)
+
+    if isinstance(error, ScriptError):
+        if error.msg:
+            print >> sys.stderr, error.msg
+        sys.exit(error.rval)
+
+    if isinstance(error, socket.error):
+        print >> sys.stderr, error
+        sys.exit(1)
+
+    if isinstance(error, ldap.INVALID_CREDENTIALS):
+        print >> sys.stderr, "Invalid password"
+        sys.exit(1)
+    if isinstance(error, ldap.INSUFFICIENT_ACCESS):
+        print >> sys.stderr, "Insufficient access"
+        sys.exit(1)
+    if isinstance(error, ldap.LOCAL_ERROR):
+        print >> sys.stderr, error.args[0]['info']
+        sys.exit(1)
+    if isinstance(error, ldap.SERVER_DOWN):
+        print >> sys.stderr, error.args[0]['desc']
+        sys.exit(1)
+    if isinstance(error, ldap.LDAPError):
+        print >> sys.stderr, 'LDAP error: %s' % type(error).__name__
+        print >> sys.stderr, error.args[0]['info']
+        sys.exit(1)
+
+    if isinstance(error, config.IPAConfigError):
+        print >> sys.stderr, "An IPA server to update cannot be found. Has one been configured yet?"
+        print >> sys.stderr, "The error was: %s" % error
+        sys.exit(1)
+    if isinstance(error, errors.LDAPError):
+        print >> sys.stderr, "An error occurred while performing operations: %s" % error
+        sys.exit(1)
+
+    if isinstance(error, HostnameLocalhost):
+        print >> sys.stderr, "The hostname resolves to the localhost address (127.0.0.1/::1)"
+        print >> sys.stderr, "Please change your /etc/hosts file so that the hostname"
+        print >> sys.stderr, "resolves to the ip address of your network interface."
+        print >> sys.stderr, ""
+        print >> sys.stderr, "Please fix your /etc/hosts file and restart the setup program"
+        sys.exit(1)
+
+    if log_file_name:
+        print >> sys.stderr, "Unexpected error - see %s for details:" % log_file_name
+    else:
+        print >> sys.stderr, "Unexpected error"
+    print >> sys.stderr, '%s: %s' % (type(error).__name__, error)
+    sys.exit(1)
diff --git a/ipaserver/install/ldapupdate.py b/ipaserver/install/ldapupdate.py
index 61a2ae19ffad0abbe9222c68190dfcac9e472c57..e75ee804a1d201512f2770156eef73cf1bb1e7bb 100644
--- a/ipaserver/install/ldapupdate.py
+++ b/ipaserver/install/ldapupdate.py
@@ -47,9 +47,12 @@
 from ipaserver.install.plugins import PRE_UPDATE, POST_UPDATE
 from ipaserver.install.plugins import FIRST, MIDDLE, LAST
 
-class BadSyntax(Exception):
+class BadSyntax(installutils.ScriptError):
     def __init__(self, value):
         self.value = value
+        self.msg = "There is a syntax error in this update file: \n  %s" % value
+        self.rval = 1
+
     def __str__(self):
         return repr(self.value)
 
-- 
1.7.10.2

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to