Re: [Freeipa-devel] [PATCH] 088 Check IPA configuration in install tools
On Fri, 2011-07-15 at 10:14 -0400, Rob Crittenden wrote: Martin Kosek wrote: On Wed, 2011-06-22 at 18:03 -0400, Rob Crittenden wrote: Martin Kosek wrote: Install tools may fail with unexpected error when IPA server is not installed on a system. Improve user experience by implementing a check to affected tools. https://fedorahosted.org/freeipa/ticket/1327 https://fedorahosted.org/freeipa/ticket/1347 Can you add a docstring to the check_server_configuration() function? Looking in each utility it isn't necessarily obvious what this does but my meager attempts at renaming it all failed. I considered is_server_installed() but that implies it would return True/False. Then I considered require_server_configured() but that didn't seem to fit either. We have lots of other check_* so I guess it is fine, but some docs on where/why it is used would be nice. rob I see you undertake the same function naming dilemma as I do. I improved documentation for the function, it should help. Martin ACK Merged to current master. Pushed to master, ipa-2-0. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 088 Check IPA configuration in install tools
On 18.7.2011 09:41, Martin Kosek wrote: On Fri, 2011-07-15 at 10:14 -0400, Rob Crittenden wrote: Martin Kosek wrote: On Wed, 2011-06-22 at 18:03 -0400, Rob Crittenden wrote: Martin Kosek wrote: Install tools may fail with unexpected error when IPA server is not installed on a system. Improve user experience by implementing a check to affected tools. https://fedorahosted.org/freeipa/ticket/1327 https://fedorahosted.org/freeipa/ticket/1347 Can you add a docstring to the check_server_configuration() function? Looking in each utility it isn't necessarily obvious what this does but my meager attempts at renaming it all failed. I considered is_server_installed() but that implies it would return True/False. Then I considered require_server_configured() but that didn't seem to fit either. We have lots of other check_* so I guess it is fine, but some docs on where/why it is used would be nice. rob I see you undertake the same function naming dilemma as I do. I improved documentation for the function, it should help. Martin ACK Merged to current master. Pushed to master, ipa-2-0. Martin I've just tried to build current master and got this: ./make-lint install/tools/ipa-replica-prepare:68: [E0602, parse_options] Undefined variable 'config' Does anyone run make-lint before submitting a patch or during review at all? :( Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 088 Check IPA configuration in install tools
On Mon, 2011-07-18 at 14:35 +0200, Jan Cholasta wrote: On 18.7.2011 09:41, Martin Kosek wrote: On Fri, 2011-07-15 at 10:14 -0400, Rob Crittenden wrote: Martin Kosek wrote: On Wed, 2011-06-22 at 18:03 -0400, Rob Crittenden wrote: Martin Kosek wrote: Install tools may fail with unexpected error when IPA server is not installed on a system. Improve user experience by implementing a check to affected tools. https://fedorahosted.org/freeipa/ticket/1327 https://fedorahosted.org/freeipa/ticket/1347 Can you add a docstring to the check_server_configuration() function? Looking in each utility it isn't necessarily obvious what this does but my meager attempts at renaming it all failed. I considered is_server_installed() but that implies it would return True/False. Then I considered require_server_configured() but that didn't seem to fit either. We have lots of other check_* so I guess it is fine, but some docs on where/why it is used would be nice. rob I see you undertake the same function naming dilemma as I do. I improved documentation for the function, it should help. Martin ACK Merged to current master. Pushed to master, ipa-2-0. Martin I've just tried to build current master and got this: ./make-lint install/tools/ipa-replica-prepare:68: [E0602, parse_options] Undefined variable 'config' Does anyone run make-lint before submitting a patch or during review at all? :( Honza We don't - so that you can rant on the list :-) Of course we do, but this one slipped in. Thanks for catching this. Fixed and pushed under the one-liner rule (patch attached). Martin From 958e8ac090e148f5d7f8c004e8e39aee3804d1ec Mon Sep 17 00:00:00 2001 From: Martin Kosek mko...@redhat.com Date: Mon, 18 Jul 2011 14:50:05 +0200 Subject: [PATCH] Fix typo in ipa-replica-prepare https://fedorahosted.org/freeipa/ticket/1327 https://fedorahosted.org/freeipa/ticket/1347 --- install/tools/ipa-replica-prepare |1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/install/tools/ipa-replica-prepare b/install/tools/ipa-replica-prepare index 14ee539135f0187d576516d640f885eec3602d8a..0c88244b33f46aa87f4f619a0b7053ec14fd7603 100755 --- a/install/tools/ipa-replica-prepare +++ b/install/tools/ipa-replica-prepare @@ -65,7 +65,6 @@ def parse_options(): default=True, help=disables pkinit setup steps) options, args = parser.parse_args() -config.init_config() if not options.ip_address: if options.reverse_zone: -- 1.7.6 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 088 Check IPA configuration in install tools
On 18.7.2011 15:00, Martin Kosek wrote: On Mon, 2011-07-18 at 14:35 +0200, Jan Cholasta wrote: On 18.7.2011 09:41, Martin Kosek wrote: On Fri, 2011-07-15 at 10:14 -0400, Rob Crittenden wrote: Martin Kosek wrote: On Wed, 2011-06-22 at 18:03 -0400, Rob Crittenden wrote: Martin Kosek wrote: Install tools may fail with unexpected error when IPA server is not installed on a system. Improve user experience by implementing a check to affected tools. https://fedorahosted.org/freeipa/ticket/1327 https://fedorahosted.org/freeipa/ticket/1347 Can you add a docstring to the check_server_configuration() function? Looking in each utility it isn't necessarily obvious what this does but my meager attempts at renaming it all failed. I considered is_server_installed() but that implies it would return True/False. Then I considered require_server_configured() but that didn't seem to fit either. We have lots of other check_* so I guess it is fine, but some docs on where/why it is used would be nice. rob I see you undertake the same function naming dilemma as I do. I improved documentation for the function, it should help. Martin ACK Merged to current master. Pushed to master, ipa-2-0. Martin I've just tried to build current master and got this: ./make-lint install/tools/ipa-replica-prepare:68: [E0602, parse_options] Undefined variable 'config' Does anyone run make-lint before submitting a patch or during review at all? :( Honza We don't - so that you can rant on the list :-) Of course we do, but this one slipped in. Thanks for catching this. Fixed and pushed under the one-liner rule (patch attached). Martin That's a relief, I got frightened for a moment :-) Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 088 Check IPA configuration in install tools
Martin Kosek wrote: On Wed, 2011-06-22 at 18:03 -0400, Rob Crittenden wrote: Martin Kosek wrote: Install tools may fail with unexpected error when IPA server is not installed on a system. Improve user experience by implementing a check to affected tools. https://fedorahosted.org/freeipa/ticket/1327 https://fedorahosted.org/freeipa/ticket/1347 Can you add a docstring to the check_server_configuration() function? Looking in each utility it isn't necessarily obvious what this does but my meager attempts at renaming it all failed. I considered is_server_installed() but that implies it would return True/False. Then I considered require_server_configured() but that didn't seem to fit either. We have lots of other check_* so I guess it is fine, but some docs on where/why it is used would be nice. rob I see you undertake the same function naming dilemma as I do. I improved documentation for the function, it should help. Martin ACK ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 088 Check IPA configuration in install tools
On Wed, 2011-06-22 at 18:03 -0400, Rob Crittenden wrote: Martin Kosek wrote: Install tools may fail with unexpected error when IPA server is not installed on a system. Improve user experience by implementing a check to affected tools. https://fedorahosted.org/freeipa/ticket/1327 https://fedorahosted.org/freeipa/ticket/1347 Can you add a docstring to the check_server_configuration() function? Looking in each utility it isn't necessarily obvious what this does but my meager attempts at renaming it all failed. I considered is_server_installed() but that implies it would return True/False. Then I considered require_server_configured() but that didn't seem to fit either. We have lots of other check_* so I guess it is fine, but some docs on where/why it is used would be nice. rob I see you undertake the same function naming dilemma as I do. I improved documentation for the function, it should help. Martin From c5ede07c01d57c3cb33f80cc0b2057d2298b0e66 Mon Sep 17 00:00:00 2001 From: Martin Kosek mko...@redhat.com Date: Wed, 22 Jun 2011 13:10:15 +0200 Subject: [PATCH] Check IPA configuration in install tools Install tools may fail with unexpected error when IPA server is not installed on a system. Improve user experience by implementing a check to affected tools. https://fedorahosted.org/freeipa/ticket/1327 https://fedorahosted.org/freeipa/ticket/1347 --- install/tools/ipa-compliance | 14 +++--- install/tools/ipa-dns-install|3 +++ install/tools/ipa-ldap-updater |7 ++- install/tools/ipa-nis-manage |2 ++ install/tools/ipa-replica-manage |7 +++ install/tools/ipa-replica-prepare|4 install/tools/ipa-server-certinstall | 13 - ipaserver/install/installutils.py| 20 ++-- 8 files changed, 59 insertions(+), 11 deletions(-) diff --git a/install/tools/ipa-compliance b/install/tools/ipa-compliance index e1de25283ba00761f3fbe363f2323fa2caa2076a..47a515bfc7b0244e3b1c2b50b5b0bb1ccc6013d0 100644 --- a/install/tools/ipa-compliance +++ b/install/tools/ipa-compliance @@ -35,6 +35,7 @@ try: from ipaserver.plugins.ldap2 import ldap2 from ipalib import api, errors, backend +from ipaserver.install import installutils except ImportError, e: # If python-rhsm isn't installed exit gracefully and quietly. if e.args[0] == 'No module named rhsm.certificate': @@ -165,8 +166,7 @@ def check_compliance(tmpdir, debug=False): print 'IPA is in compliance: %d of %d entitlements used.' % (hostcount, available) def main(): -if os.getegid() != 0: -sys.exit(Must be root to check compliance) +installutils.check_server_configuration() if not os.path.exists('/etc/ipa/default.conf'): return 0 @@ -189,4 +189,12 @@ def main(): return 0 -sys.exit(main()) +try: +if not os.geteuid()==0: +sys.exit(\nMust be root to check compliance\n) + +main() +except SystemExit, e: +sys.exit(e) +except RuntimeError, e: +sys.exit(e) diff --git a/install/tools/ipa-dns-install b/install/tools/ipa-dns-install index b5295b5c7567c3d559ad808d1752d79c1d915573..16cc33b38fbe842fdf2b5a82ca42601d6a9ab0ac 100755 --- a/install/tools/ipa-dns-install +++ b/install/tools/ipa-dns-install @@ -24,6 +24,7 @@ import traceback from ipaserver.plugins.ldap2 import ldap2 from ipaserver.install import bindinstance, ntpinstance from ipaserver.install.installutils import * +from ipaserver.install import installutils from ipapython import version from ipapython import ipautil, sysrestore from ipalib import api, errors, util @@ -69,6 +70,8 @@ def main(): if os.getegid() != 0: sys.exit(Must be root to setup server) +installutils.check_server_configuration() + standard_logging_setup(/var/log/ipaserver-install.log, options.debug, filemode='a') print \nThe log file for this installation can be found in /var/log/ipaserver-install.log diff --git a/install/tools/ipa-ldap-updater b/install/tools/ipa-ldap-updater index ec57109d3c03fae11c8028edd8d20a88e40cc02a..5b63c120ec83a03fa0cc7ba9aab1cb60bda23e31 100755 --- a/install/tools/ipa-ldap-updater +++ b/install/tools/ipa-ldap-updater @@ -85,9 +85,7 @@ def main(): loglevel = logging.DEBUG if os.getegid() == 0: -fstore = sysrestore.FileStore('/var/lib/ipa/sysrestore') -if not fstore.has_files(): -sys.exit(IPA is not configured on this system.) +installutils.check_server_configuration() elif not os.path.exists('/etc/ipa/default.conf'): sys.exit(IPA is not configured on this system.) @@ -149,8 +147,7 @@ except BadSyntax, e: print %s % e sys.exit(1) except RuntimeError, e: -print %s % e -sys.exit(1) +sys.exit(e) except SystemExit, e: sys.exit(e) except KeyboardInterrupt, e: diff --git a/install/tools/ipa-nis-manage b/install/tools/ipa-nis-manage index