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
