On 04/17/2012 12:12 AM, Rob Crittenden wrote:
John Dennis wrote:
On 04/16/2012 04:15 PM, Rob Crittenden wrote:
John Dennis wrote:
On 04/16/2012 01:31 PM, Rob Crittenden wrote:
John Dennis wrote:
On 04/13/2012 06:25 AM, Petr Viktorin wrote:
When the utility sets logging to console, the extra log message
gets
printed out there. I agree this isn't optimal.
Attached patch removes the console log handler before logging the
result.


I read through log_manager, and found that I can do this more
cleanly
with remove_handler.

John, is this a good use of the API?

The problem you're trying to correct is that under some
circumstances a
log message is emitted twice to the console, right?

Removing the console handler fees like the wrong solution, it's a
pretty
big hammer, you have to know when to remove it and once removed any
expectation that messages will appear on the console will be untrue.

Can you give me a brief explanation as to why you're getting
duplicate
messages on the console. I wonder if there isn't a better way to
handle
the problem which isn't so invasive and potentially hidden.

The patch adds a final log message that says if the command ultimately succeeded or not (https://fedorahosted.org/freeipa/ticket/2071). Some of the commands print an error message to the console independently. When they've configured logging to console, the added message appears as well.

The right thing to do is to modify the utility just log once. But this patch treats the utilities as black boxes, otherwise it's too invasive.

Or is the issue you don't want a console handler at all? If that's
the
case then maybe we should provide a configuration that does not
create
one, that way it will be explicit and obvious there is no console
handler.

This would mean changing how the commands set up logging. I plan to do that, but it's too invasive to do now.

FWIW, the current configuration of logging is historical dating
back to
the beginning of the project. When I added the log manager the intent
was to fix deficiencies in logging, not to modify the existing
behavior.
I wasn't clear to me the existing configuration was ideal. If you're
hitting problems because of the existing configuration perhaps we
should
look at the historical configuration and ask if it needs to be
modified.

This patch is standardizing logging the final disposition of a
number of
commands. Currently is must be inferred.

The problem is that some commands have had this disposition added but
open no log files so some error messages are being displayed twice and
in log format.

I think the easiest solution would simply be to scale this back to
only
those commands that open a log file at all.

You say "command" but command is a loaded word :-) I presume you mean
command line utility and not an IPA Command object. The reason I'm
drawing this distinction is because the environment Commands execute in
are not supposed to change once api.bootstrap() has completed.

Who or what is aggregating the final disposition of a number of
commands? The reason I ask is because the log_mgr object is global and
deleting a handler from a global resource to satisfy a local need may
have unintentional global side effects. How is the aggregation
accomplished?

This is a wrapper around scripts. The final log message is the last thing done before exiting Python.

This patch is in the context of the command-line utilities like
ipa-server-install, ipa-client-install, etc. and not the ipa tool. Some
of them initialize the api, some do not.

Also there's ipa-ldap-updater, which sometimes opens a log file and sometimes not.

This is exactly the type of problem I'm concerned about, a conflict over
who owns the logging configuration.

These multiple potential "owners" of a global configuration is what lead
me to introduce the "configure_state" attribute of the log manager, to
disambiguate who is in control and who initialized the configuration. In
fact bootstrap() in plugable.py explicitly examines the configure_state
attribute of the log_manager and if it's been previously configured (for
example because the api was loaded by a command line utility that has
configured it's own logging) it then defers to the logging configuration
established by the environment it's being run in (in other words if a
command line utility calls standard_logging_setup() before loading the
api it wins and the logging configuration belongs to the utility, not
the api).

All of this is to say that isolated code that reaches into a global
configuration and takes an axe to it and chops out a significant part of
the configuration that someone else may have established without them
knowing about it feels wrong.

I suspect there is some deeper problem afoot and unilaterally chopping
out the console handler is not addressing the fundamental problem and
likely introduces a new one. But I really need to look at the specific
instances of the problem, maybe if I dig through the history of this
patch review more carefully I'll discover it, but if someone wants to
chime in an point me at the exact issue that would be great :-)

Like I've said, this is a non-issue in this case. Those utilities that
do not open a log simply don't need to call this log_on_exit function.

Any remaining logging issues should be logged as a ticket.

I think you're overstating the logging problem in general though.
Basically we just want a way to override the default log that the API
chooses. So the API bootstrap() call defers to the user if logging has
already been configured. It is the responsibility of the developer to
know when to do what but since there is less than a half dozen
exceptions I don't think this is worth a significant time investment.

At some point we may rewrite all of these utilities to use the API
itself at which time this problem will simply go away.

rob

Attaching a patch that only adds the final log for commands that do standard_logging_setup. It still includes the removing of the console handler, though.


--
PetrĀ³
From 5c715f74b364276edf35f237248a26ed8fd80636 Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pvikt...@redhat.com>
Date: Thu, 1 Mar 2012 05:29:52 -0500
Subject: [PATCH] Add final debug message in installers

Invocation of install tools is wrapped in a context manager that logs
the final success or failure.

https://fedorahosted.org/freeipa/ticket/2071
---
 install/tools/ipa-adtrust-install   |   47 ++++++++++++-----------
 install/tools/ipa-ca-install        |   71 ++++++++++++++++++-----------------
 install/tools/ipa-compat-manage     |   43 +++++++++++----------
 install/tools/ipa-dns-install       |   47 ++++++++++++-----------
 install/tools/ipa-ldap-updater      |   27 +++++++------
 install/tools/ipa-managed-entries   |   35 +++++++++--------
 install/tools/ipa-nis-manage        |   43 +++++++++++----------
 install/tools/ipa-replica-conncheck |   35 +++++++++--------
 install/tools/ipa-replica-install   |   71 ++++++++++++++++++-----------------
 install/tools/ipa-server-install    |   66 +++++++++++++++++----------------
 install/tools/ipa-upgradeconfig     |    1 +
 ipaserver/install/installutils.py   |   24 ++++++++++++
 12 files changed, 273 insertions(+), 237 deletions(-)

diff --git a/install/tools/ipa-adtrust-install b/install/tools/ipa-adtrust-install
index 248ea35eaa86dd59ebbc871b86df780cfd71ccf6..c3558fce0cceae879f83b61e0057d14fe42811de 100755
--- a/install/tools/ipa-adtrust-install
+++ b/install/tools/ipa-adtrust-install
@@ -227,26 +227,27 @@ 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)
+with installutils.script_context('ipa-adtrust-install'):
+    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)
diff --git a/install/tools/ipa-ca-install b/install/tools/ipa-ca-install
index 57f867e704b559a691ac48a564c152e3b98def72..48a0a4b39dcd18ae0e8c1ebb5fef328e3dc60b8a 100755
--- a/install/tools/ipa-ca-install
+++ b/install/tools/ipa-ca-install
@@ -150,41 +150,42 @@ 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")
+with installutils.script_context('ipa-ca-install'):
+    try:
+        if not os.geteuid()==0:
+            sys.exit("\nYou must be root to run this script.\n")
+
+        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
+        try:
+            if REPLICA_INFO_TOP_DIR:
+                shutil.rmtree(REPLICA_INFO_TOP_DIR)
+        except OSError:
+            pass
 
-    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
-    try:
-        if REPLICA_INFO_TOP_DIR:
-            shutil.rmtree(REPLICA_INFO_TOP_DIR)
-    except OSError:
-        pass
+    print "Your system may be partly configured."
+    print "Run /usr/sbin/ipa-server-install --uninstall to clean up."
 
-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)
+    # the only way to get here is on error or ^C
+    sys.exit(1)
diff --git a/install/tools/ipa-compat-manage b/install/tools/ipa-compat-manage
index 13a93cbed02ca69f4f6e8cb156a2f6f18e2da899..48bea7dd6531cba79c46dff4061ea09d5ffa3aef 100755
--- a/install/tools/ipa-compat-manage
+++ b/install/tools/ipa-compat-manage
@@ -196,24 +196,25 @@ 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)
+with installutils.script_context('ipa-compat-manage'):
+    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)
diff --git a/install/tools/ipa-dns-install b/install/tools/ipa-dns-install
index b540630f4f2782603c31ce1905870c38c9af98ab..892b5cc792054e596e6d74902eed8573af4b67d7 100755
--- a/install/tools/ipa-dns-install
+++ b/install/tools/ipa-dns-install
@@ -243,26 +243,27 @@ 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)
+with installutils.script_context('ipa-dns-install'):
+    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)
diff --git a/install/tools/ipa-ldap-updater b/install/tools/ipa-ldap-updater
index bd2233a94241c28375b29cc10d60908238b8f176..4de69e31a217d44734a24877d4d30721904e325c 100755
--- a/install/tools/ipa-ldap-updater
+++ b/install/tools/ipa-ldap-updater
@@ -154,16 +154,17 @@ def main():
         root_logger.info('Update complete')
         return 0
 
-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:
-    sys.exit(e)
-except SystemExit, e:
-    sys.exit(e)
-except KeyboardInterrupt, e:
-    sys.exit(1)
+with installutils.script_context('ipa-ldap-updater'):
+    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:
+        sys.exit(e)
+    except SystemExit, e:
+        sys.exit(e)
+    except KeyboardInterrupt, e:
+        sys.exit(1)
diff --git a/install/tools/ipa-managed-entries b/install/tools/ipa-managed-entries
index 00bb566226adf636fe1c2cfc4a6357636f3ffb71..edc5605b8678cd73a686c68f3baa7cc8deb03168 100755
--- a/install/tools/ipa-managed-entries
+++ b/install/tools/ipa-managed-entries
@@ -237,20 +237,21 @@ 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)
+with installutils.script_context('ipa-managed-entries'):
+    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)
diff --git a/install/tools/ipa-nis-manage b/install/tools/ipa-nis-manage
index 5c5bbca8e0435441cbb2ea10d80245e36a86e9a7..5473a45ed05db5268effa680f6431d1d69a3cb35 100755
--- a/install/tools/ipa-nis-manage
+++ b/install/tools/ipa-nis-manage
@@ -200,24 +200,25 @@ 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)
+with installutils.script_context('ipa-nis-manage'):
+    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)
diff --git a/install/tools/ipa-replica-conncheck b/install/tools/ipa-replica-conncheck
index 6ec3be2a919c4a8a8a32cbf76f54b12d6652ff5e..9a8535ece8dd1b4c0c429901a0837cb3c9376929 100755
--- a/install/tools/ipa-replica-conncheck
+++ b/install/tools/ipa-replica-conncheck
@@ -387,20 +387,21 @@ def main():
             print_info("Connection check timeout: terminating listening program")
 
 if __name__ == "__main__":
-    try:
-        sys.exit(main())
-    except SystemExit, e:
-        sys.exit(e)
-    except KeyboardInterrupt:
-        print_info("\nCleaning up...")
-        sys.exit(1)
-    except RuntimeError, e:
-        sys.exit(e)
-    finally:
-        clean_responders(RESPONDERS)
-        for file_name in (CCACHE_FILE, KRB5_CONFIG):
-            if file_name:
-                try:
-                    os.remove(file_name)
-                except OSError:
-                    pass
+    with installutils.script_context('ipa-replica-conncheck'):
+        try:
+            sys.exit(main())
+        except SystemExit, e:
+            sys.exit(e)
+        except KeyboardInterrupt:
+            print_info("\nCleaning up...")
+            sys.exit(1)
+        except RuntimeError, e:
+            sys.exit(e)
+        finally:
+            clean_responders(RESPONDERS)
+            for file_name in (CCACHE_FILE, KRB5_CONFIG):
+                if file_name:
+                    try:
+                        os.remove(file_name)
+                    except OSError:
+                        pass
diff --git a/install/tools/ipa-replica-install b/install/tools/ipa-replica-install
index 07b1781ee7f99cacf1a3abbd6207b95f38da1807..d8240120181b49e335b09cd72f88f534327ac088 100755
--- a/install/tools/ipa-replica-install
+++ b/install/tools/ipa-replica-install
@@ -487,41 +487,42 @@ 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")
-
-    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
+with installutils.script_context('ipa-replica-install'):
     try:
-        if REPLICA_INFO_TOP_DIR:
-            shutil.rmtree(REPLICA_INFO_TOP_DIR)
-    except OSError:
-        pass
+        if not os.geteuid()==0:
+            sys.exit("\nYou must be root to run this script.\n")
 
-print ""
-print "Your system may be partly configured."
-print "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
+        try:
+            if REPLICA_INFO_TOP_DIR:
+                shutil.rmtree(REPLICA_INFO_TOP_DIR)
+        except OSError:
+            pass
 
-# the only way to get here is on error or ^C
-sys.exit(1)
+    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)
diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install
index 1dd02ba870a02e902c4c345d9f5802ef09f78a7a..ab86794952da2c56c67525f38205a71934a15166 100755
--- a/install/tools/ipa-server-install
+++ b/install/tools/ipa-server-install
@@ -49,6 +49,7 @@ from ipaserver.install import ntpinstance
 from ipaserver.install import certs
 from ipaserver.install import cainstance
 from ipaserver.install import memcacheinstance
+from ipaserver.install import installutils
 
 from ipaserver.install import service
 from ipapython import version
@@ -1094,37 +1095,38 @@ def main():
         os.remove(ANSWER_CACHE)
     return 0
 
-try:
-    success = True
+with installutils.script_context('ipa-server-install'):
     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" % str(e)
-        else:
-            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)
-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
+        success = True
         try:
-            fstore.restore_file('/etc/hosts')
-        except:
-            pass
+            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" % str(e)
+            else:
+                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)
+    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..3aee65d7470905959b7c68f3fceaa2e88801d622 100644
--- a/install/tools/ipa-upgradeconfig
+++ b/install/tools/ipa-upgradeconfig
@@ -305,6 +305,7 @@ def main():
     cleanup_kdc(fstore)
     upgrade_ipa_profile(krbctx.default_realm)
 
+
 try:
     if __name__ == "__main__":
         sys.exit(main())
diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py
index 3e7ae41b5fdbc11353e43a63424f19fbc331435a..04fa43d8015684da5732f7cdd63b1cb03758feed 100644
--- a/ipaserver/install/installutils.py
+++ b/ipaserver/install/installutils.py
@@ -31,6 +31,7 @@
 import tempfile
 import shutil
 from ConfigParser import SafeConfigParser
+import contextlib
 
 from ipapython import ipautil, dnsclient, sysrestore
 from ipapython.ipa_log_manager import *
@@ -712,3 +713,26 @@ def is_ipa_configured():
         root_logger.debug('filestore is tracking no files')
 
     return installed
+
+
+@contextlib.contextmanager
+def script_context(operation_name):
+    try:
+        try:
+            yield
+        finally:
+            # Do not print the extra log message to the console
+            try:
+                log_mgr.remove_handler('console')
+            except LookupError:
+                pass
+    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:
+            root_logger.error('The %s command failed, %s: %s' %
+                (operation_name, type(e).__name__, e))
+        raise
+    else:
+        root_logger.info('The %s command was successful' % operation_name)
-- 
1.7.7.6

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

Reply via email to