On Tue, 2011-05-10 at 10:06 -0400, Rob Crittenden wrote: > Martin Kosek wrote: > > Remove redundant ipa-client-install error message when optional nscd > > daemon was not installed. Additionally, use standard IPA functions > > for service manipulation and improve logging. > > > > https://fedorahosted.org/freeipa/ticket/1207 > Nack, a client-only install isn't going to have ipaserver to import from.
Good catch, I didn't realize that. Will do next time. I have moved the /sbin/service and /sbin/chkconfig control routines to ipautil library, which are called by ipa-client-install. I have left the interface in ipaserver.install.service as it used through many scripts and we could use this interface later when implementing a native systemd support. The deciding logic what init system to use use can be then hidden behind this interface. > > Ignoring certmonger not starting was for the case where it is already > running. Ideally we should check the status of the service and start it > if necessary. I think I have not touched this logic, I just added few logging statements that we can analyze when future user's will fill us bug reports :-) > > Some of this could be moved to ipapython as that is where common, > non-framework code goes. Yeah, I chose ipapython.ipautil library. Please, take a look at the attached patch. Martin
>From 8dc0b414d17cfa66bf43fe0cb37ca5de4beffcb9 Mon Sep 17 00:00:00 2001 From: Martin Kosek <mko...@redhat.com> Date: Tue, 10 May 2011 15:14:20 +0200 Subject: [PATCH] Improve service manipulation in client install Remove redundant ipa-client-install error message when optional nscd daemon was not installed. Additionally, use standard IPA functions for service manipulation and improve logging. https://fedorahosted.org/freeipa/ticket/1207 --- ipa-client/ipa-install/ipa-client-install | 157 ++++++++++++----------------- ipapython/ipautil.py | 48 +++++++++ ipaserver/install/service.py | 30 ++---- 3 files changed, 124 insertions(+), 111 deletions(-) diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install index 6265a7c2e943338103d36962a5a5d0b810c7e3d7..2bcd4b9162aabd785e9266f46a59442555143e76 100755 --- a/ipa-client/ipa-install/ipa-client-install +++ b/ipa-client/ipa-install/ipa-client-install @@ -133,50 +133,6 @@ def nickname_exists(nickname): else: return False -def service(name, status): - """ - Run a System V init script 'name' with the status 'status' - - The return value of /sbin/service name start/stop/status is: - - 0 - Ok - 1 - unrecognized service, bad usage - > 1 - generally command-specific - - For status == 'status' it means: - 0 - running - 1 - dead but pid file exists - 2 - dead but sybsys locked - 3 - stopped - """ - (sout, serr, returncode) = run(['/sbin/service', name, 'status'], raiseonerr=False) - - # If the service isn't installed return with no error - if returncode == 1: - return - - args = ['/sbin/service', name, status] - (sout, serr, returncode) = run(args, raiseonerr=False) - - if returncode != 0: - raise CalledProcessError(returncode, ' '.join(args)) - - return - -def chkconfig(name, status): - """ - Set startup of service 'name' to 'status' (on or off) - - chkconfig returns 1 if the service is unknown, 0 otherwise - """ - args = ['/sbin/chkconfig', name, status] - (sout, serr, returncode) = run(args, raiseonerr=False) - - if returncode != 0: - raise CalledProcessError(returncode, ' '.join(args)) - - return - def uninstall(options, env): if not fstore.has_files(): @@ -221,13 +177,15 @@ def uninstall(options, env): # Always start certmonger. We can't untrack something if it isn't # running try: - service('certmonger', 'start') - except: - pass + ipautil.service_start('certmonger') + except Exception, e: + logging.error("certmonger failed to start: %s" % str(e)) + try: certmonger.stop_tracking('/etc/pki/nssdb', nickname=client_nss_nickname) except (CalledProcessError, RuntimeError), e: logging.error("certmonger failed to stop tracking certificate: %s" % str(e)) + if nickname_exists(client_nss_nickname): try: run(["/usr/bin/certutil", "-D", "-d", "/etc/pki/nssdb", "-n", client_nss_nickname]) @@ -235,17 +193,18 @@ def uninstall(options, env): print "Failed to remove %s from /etc/pki/nssdb: %s" % (client_nss_nickname, str(e)) try: - service('certmonger', 'stop') - except: - pass + ipautil.service_stop('certmonger') + except Exception, e: + logging.error("certmonger failed to stop: %s" % str(e)) # Remove any special principal names we added to the IPA CA helper certmonger.remove_principal_from_cas() try: - chkconfig('certmonger', 'off') - except: + ipautil.chkconfig_off('certmonger') + except Exception, e: print "Failed to disable automatic startup of the certmonger daemon" + logging.error("Failed to disable automatic startup of the certmonger daemon: %s" % str(e)) if not options.on_master: print "Unenrolling client from IPA server" @@ -262,8 +221,9 @@ def uninstall(options, env): fp.close() realm = parser.get('global', 'realm') run(["/usr/sbin/ipa-rmkeytab", "-k", "/etc/krb5.keytab", "-r", realm]) - except: + except Exception, e: print "Failed to clean up /etc/krb5.keytab" + logging.error("Failed to remove Kerberos service principals: %s" % str(e)) print "Disabling client Kerberos and LDAP configurations" try: @@ -275,15 +235,19 @@ def uninstall(options, env): print "Restoring client configuration files" fstore.restore_all_files() - try: - service('nscd', 'restart') - except: - print "Failed to restart start the NSCD daemon" - - try: - chkconfig('nscd', 'on') - except: - print "Failed to configure automatic startup of the NSCD daemon" + if ipautil.service_is_installed('nscd'): + try: + ipautil.service_restart('nscd') + except: + print "Failed to restart start the NSCD daemon" + + try: + ipautil.chkconfig_on('nscd') + except: + print "Failed to configure automatic startup of the NSCD daemon" + else: + # this is optional service, just log + logging.info("NSCD daemon is not installed, skip configuration") if not options.unattended: print "The original nsswitch.conf configuration has been restored." @@ -491,33 +455,34 @@ def configure_certmonger(fstore, subject_base, cli_realm, hostname, options): # Ensure that certmonger has been started at least once to generate the # cas files in /var/lib/certmonger/cas. try: - service('certmonger', 'restart') - except: - pass - + ipautil.service_restart('certmonger') + except Exception, e: + logging.error("certmonger failed to restart: %s" % str(e)) if options.hostname: # It needs to be stopped if we touch them try: - service('certmonger', 'stop') - except: - pass + ipautil.service_stop('certmonger') + except Exception, e: + logging.error("certmonger failed to stop: %s" % str(e)) # If the hostname is explicitly set then we need to tell certmonger # which principal name to use when requesting certs. certmonger.add_principal_to_cas(principal) try: - service('certmonger', 'restart') - except: + ipautil.service_restart('certmonger') + except Exception, e: print "Failed to start the certmonger daemon" print "Automatic certificate management will not be available" + logging.error("certmonger failed to restart: %s" % str(e)) started = False try: - chkconfig('certmonger', 'on') - except: + ipautil.chkconfig_on('certmonger') + except Exception, e: print "Failed to configure automatic startup of the certmonger daemon" print "Automatic certificate management will not be available" + logging.error("Failed to disable automatic startup of the certmonger daemon: %s" % str(e)) # Request our host cert if started: @@ -910,27 +875,33 @@ def main(): if not options.on_master: client_dns(cli_server, hostname, options.dns_updates) - if options.sssd: - nscd_action = "stop" - nscd_status = "off" + #Name Server Caching Daemon. Disable for SSSD, use otherwise (if installed) + if ipautil.service_is_installed("nscd"): + if options.sssd: + nscd_service_action = "stop" + nscd_service_cmd = ipautil.service_stop + nscd_chkconfig_cmd = ipautil.chkconfig_off + else: + nscd_service_action = "restart" + nscd_service_cmd = ipautil.service_restart + nscd_chkconfig_cmd = ipautil.chkconfig_on + + try: + nscd_service_cmd('nscd') + except: + print >>sys.stderr, "Failed to %s the NSCD daemon" % nscd_service_action + if not options.sssd: + print >>sys.stderr, "Caching of users/groups will not be available" + + try: + nscd_chkconfig_cmd('nscd') + except: + print >>sys.stderr, "Failed to configure automatic startup of the NSCD daemon" + if not options.sssd: + print >>sys.stderr, "Caching of users/groups will not be available after reboot" else: - nscd_action = "restart" - nscd_status = "on" - - #Name Server Caching Daemon. Disable for SSSD, use otherwise - try: - service('nscd', nscd_action) - except: - print >>sys.stderr, "Failed to %s the NSCD daemon" % nscd_action - if not options.sssd: - print >>sys.stderr, "Caching of users/groups will not be available" - - try: - chkconfig('nscd', nscd_status) - except: - print >>sys.stderr, "Failed to configure automatic startup of the NSCD daemon" - if not options.sssd: - print >>sys.stderr, "Caching of users/groups will not be available after reboot" + # this is optional service, just log + logging.info("NSCD daemon is not installed, skip configuration") # Modify nsswitch/pam stack if options.sssd: diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py index b5a0b9105800b55296bb36877e9b98666426c7cc..4280cd9f4f8ff6a98f5ff9808d8a9c6cfe8df75b 100644 --- a/ipapython/ipautil.py +++ b/ipapython/ipautil.py @@ -967,3 +967,51 @@ def get_gsserror(e): minor = e[0][1] return (major, minor) + + +def service_stop(service_name, instance_name="", capture_output=True): + run(["/sbin/service", service_name, "stop", instance_name], + capture_output=capture_output) + +def service_start(service_name, instance_name="", capture_output=True): + run(["/sbin/service", service_name, "start", instance_name], + capture_output=capture_output) + +def service_restart(service_name, instance_name="", capture_output=True): + run(["/sbin/service", service_name, "restart", instance_name], + capture_output=capture_output) + +def service_is_running(service_name, instance_name=""): + ret = True + try: + run(["/sbin/service", service_name, "status", instance_name]) + except CalledProcessError: + ret = False + return ret + +def service_is_installed(service_name): + installed = True + try: + run(["/sbin/service", service_name, "status"]) + except CalledProcessError, e: + if e.returncode == 1: + # service is not installed or there is other serious issue + installed = False + return installed + +def service_is_enabled(service_name): + (stdout, stderr, returncode) = run(["/sbin/chkconfig", service_name], raiseonerr=False) + return (returncode == 0) + +def chkconfig_on(service_name): + run(["/sbin/chkconfig", service_name, "on"]) + +def chkconfig_off(service_name): + run(["/sbin/chkconfig", service_name, "off"]) + +def chkconfig_add(service_name): + run(["/sbin/chkconfig", "--add", service_name]) + +def chkconfig_del(service_name): + run(["/sbin/chkconfig", "--del", service_name]) + diff --git a/ipaserver/install/service.py b/ipaserver/install/service.py index 0d31927009df084049c36a1e3c9d0b7d3c6511da..d8d04e73aa305e9490927ac80e25ff99309c1da4 100644 --- a/ipaserver/install/service.py +++ b/ipaserver/install/service.py @@ -41,40 +41,34 @@ SERVICE_LIST = { } def stop(service_name, instance_name="", capture_output=True): - ipautil.run(["/sbin/service", service_name, "stop", instance_name], - capture_output=capture_output) + ipautil.service_stop(service_name, instance_name, capture_output) def start(service_name, instance_name="", capture_output=True): - ipautil.run(["/sbin/service", service_name, "start", instance_name], - capture_output=capture_output) + ipautil.service_start(service_name, instance_name, capture_output) def restart(service_name, instance_name="", capture_output=True): - ipautil.run(["/sbin/service", service_name, "restart", instance_name], - capture_output=capture_output) + ipautil.service_restart(service_name, instance_name, capture_output) def is_running(service_name, instance_name=""): - ret = True - try: - ipautil.run(["/sbin/service", service_name, "status", instance_name]) - except ipautil.CalledProcessError: - ret = False - return ret + return ipautil.service_is_running(service_name, instance_name) + +def is_installed(service_name): + return ipautil.service_is_installed(service_name) def chkconfig_on(service_name): - ipautil.run(["/sbin/chkconfig", service_name, "on"]) + ipautil.chkconfig_on(service_name) def chkconfig_off(service_name): - ipautil.run(["/sbin/chkconfig", service_name, "off"]) + ipautil.chkconfig_on(service_name) def chkconfig_add(service_name): - ipautil.run(["/sbin/chkconfig", "--add", service_name]) + ipautil.chkconfig_on(service_name) def chkconfig_del(service_name): - ipautil.run(["/sbin/chkconfig", "--del", service_name]) + ipautil.chkconfig_on(service_name) def is_enabled(service_name): - (stdout, stderr, returncode) = ipautil.run(["/sbin/chkconfig", service_name], raiseonerr=False) - return (returncode == 0) + return ipautil.service_is_enabled(service_name) def print_msg(message, output_fd=sys.stdout): logging.debug(message) -- 1.7.4.4
_______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
