Re: [Freeipa-devel] [PATCH] 500 Fix shutdown issues with systemd
On Fri, 2012-10-19 at 14:15 -0400, Rob Crittenden wrote:
Simo Sorce wrote:
On Fri, 2012-10-19 at 13:47 -0400, Rob Crittenden wrote:
Simo Sorce wrote:
On Thu, 2012-10-18 at 11:51 -0400, Rob Crittenden wrote:
Simo Sorce wrote:
On Thu, 2012-10-18 at 11:37 -0400, Rob Crittenden wrote:
Simo Sorce wrote:
Also improve shutdown reliability and restart behavior so we always
kill
all the processes we started even if the list of processes to handle
changed in LDAP.
Fixes: https://fedorahosted.org/freeipa/ticket/2302
Should this list be updated if we do a post-install of DNS or the CA?
It
isn't now which would leave some services running.
Yes we probably should,
but I think it should be done as a separate ticket.
Simo.
What's the impact if we don't do this now?
some services will not be turned off by ipactl, however this shouldn't
impact much at shutdown, as systemd will shutdown left over stuff as it
keeps track of all processes.
uhmm however if someone does a ipactl restart those services will not be
restarted the first time it is run (we will tell systemd to start them
but it will do likely nothing as they are already started), after that
they will be handled as they fact they are enabled will be stored in the
file.
Well, what is the downside of a call to refresh the list of running
services that should be run at the end of setting up new services?
Otherwise we could find outselves with a bunch of unreproducible cases
where things weren't start/stopped properly.
Yeah I have been thinking about that.
I am thinking of changing the patch so that it's the service class
itself that saves this data.
That way saving is always performed no matter 'what' starts the service.
Do you agree ?
Yes, that sounds like it'll be very robust.
Ok I created 3 new patches that replace the previous one.
The first 2 patches lay groundwork to have the service class save in the
services.list file.
The last one changes ipactl to actually use the file.
Simo.
--
Simo Sorce * Red Hat, Inc * New York
From ef39621897d3bced7f4bf41ab9ac04a266bf9fa3 Mon Sep 17 00:00:00 2001
From: Simo Sorce sso...@redhat.com
Date: Mon, 22 Oct 2012 16:59:43 -0400
Subject: [PATCH 1/3] Preserve original service_name in services
This is needed to be able to reference stuff always wth the same name.
The platform specific private name must be kept in a platform specific
variable.
In the case of systemd we store it in systemd_name
For the redhat platform wellknown names and service name are the same so
currently no special name is needed.
---
ipapython/platform/fedora16.py | 11 +--
ipapython/platform/systemd.py | 19 ++-
2 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/ipapython/platform/fedora16.py b/ipapython/platform/fedora16.py
index 794c39e2091f9402282e18fbe162d40892cb1e0d..ced85ce593dd31655dcbbd8a6d1900cee7b9e9ea 100644
--- a/ipapython/platform/fedora16.py
+++ b/ipapython/platform/fedora16.py
@@ -68,15 +68,14 @@ system_units['pki_tomcatd'] = system_units['pki-tomcatd']
class Fedora16Service(systemd.SystemdService):
def __init__(self, service_name):
if service_name in system_units:
-service_name = system_units[service_name]
+systemd_name = system_units[service_name]
else:
if len(service_name.split('.')) == 1:
# if service_name does not have a dot, it is not foo.service
# and not a foo.target. Thus, not correct service name for
# systemd, default to foo.service style then
-service_name = %s.service % (service_name)
-super(Fedora16Service, self).__init__(service_name)
-
+systemd_name = %s.service % (service_name)
+super(Fedora16Service, self).__init__(service_name, systemd_name)
# Special handling of directory server service
#
# We need to explicitly enable instances to install proper symlinks as
@@ -104,8 +103,8 @@ class Fedora16DirectoryService(Fedora16Service):
def restart(self, instance_name=, capture_output=True, wait=True):
if len(instance_name) 0:
-elements = self.service_name.split(@)
-srv_etc = os.path.join(self.SYSTEMD_ETC_PATH, self.service_name)
+elements = self.systemd_name.split(@)
+srv_etc = os.path.join(self.SYSTEMD_ETC_PATH, self.systemd_name)
srv_tgt = os.path.join(self.SYSTEMD_ETC_PATH, self.SYSTEMD_SRV_TARGET % (elements[0]))
srv_lnk = os.path.join(srv_tgt, self.service_instance(instance_name))
if not os.path.exists(srv_etc):
diff --git a/ipapython/platform/systemd.py b/ipapython/platform/systemd.py
index c174488c08a73ce02b5f568ddd24c98d8dab83d1..6c25a79b6ecdfbda1c85ada6642a656d704fdb2d 100644
--- a/ipapython/platform/systemd.py
+++ b/ipapython/platform/systemd.py
@@ -27,25 +27,26 @@ class SystemdService(base.PlatformService):
Re: [Freeipa-devel] [PATCH] 500 Fix shutdown issues with systemd
Simo Sorce wrote: On Thu, 2012-10-18 at 11:51 -0400, Rob Crittenden wrote: Simo Sorce wrote: On Thu, 2012-10-18 at 11:37 -0400, Rob Crittenden wrote: Simo Sorce wrote: Also improve shutdown reliability and restart behavior so we always kill all the processes we started even if the list of processes to handle changed in LDAP. Fixes: https://fedorahosted.org/freeipa/ticket/2302 Should this list be updated if we do a post-install of DNS or the CA? It isn't now which would leave some services running. Yes we probably should, but I think it should be done as a separate ticket. Simo. What's the impact if we don't do this now? some services will not be turned off by ipactl, however this shouldn't impact much at shutdown, as systemd will shutdown left over stuff as it keeps track of all processes. uhmm however if someone does a ipactl restart those services will not be restarted the first time it is run (we will tell systemd to start them but it will do likely nothing as they are already started), after that they will be handled as they fact they are enabled will be stored in the file. Well, what is the downside of a call to refresh the list of running services that should be run at the end of setting up new services? Otherwise we could find outselves with a bunch of unreproducible cases where things weren't start/stopped properly. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 500 Fix shutdown issues with systemd
On Fri, 2012-10-19 at 13:47 -0400, Rob Crittenden wrote: Simo Sorce wrote: On Thu, 2012-10-18 at 11:51 -0400, Rob Crittenden wrote: Simo Sorce wrote: On Thu, 2012-10-18 at 11:37 -0400, Rob Crittenden wrote: Simo Sorce wrote: Also improve shutdown reliability and restart behavior so we always kill all the processes we started even if the list of processes to handle changed in LDAP. Fixes: https://fedorahosted.org/freeipa/ticket/2302 Should this list be updated if we do a post-install of DNS or the CA? It isn't now which would leave some services running. Yes we probably should, but I think it should be done as a separate ticket. Simo. What's the impact if we don't do this now? some services will not be turned off by ipactl, however this shouldn't impact much at shutdown, as systemd will shutdown left over stuff as it keeps track of all processes. uhmm however if someone does a ipactl restart those services will not be restarted the first time it is run (we will tell systemd to start them but it will do likely nothing as they are already started), after that they will be handled as they fact they are enabled will be stored in the file. Well, what is the downside of a call to refresh the list of running services that should be run at the end of setting up new services? Otherwise we could find outselves with a bunch of unreproducible cases where things weren't start/stopped properly. Yeah I have been thinking about that. I am thinking of changing the patch so that it's the service class itself that saves this data. That way saving is always performed no matter 'what' starts the service. Do you agree ? Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 500 Fix shutdown issues with systemd
Simo Sorce wrote: On Fri, 2012-10-19 at 13:47 -0400, Rob Crittenden wrote: Simo Sorce wrote: On Thu, 2012-10-18 at 11:51 -0400, Rob Crittenden wrote: Simo Sorce wrote: On Thu, 2012-10-18 at 11:37 -0400, Rob Crittenden wrote: Simo Sorce wrote: Also improve shutdown reliability and restart behavior so we always kill all the processes we started even if the list of processes to handle changed in LDAP. Fixes: https://fedorahosted.org/freeipa/ticket/2302 Should this list be updated if we do a post-install of DNS or the CA? It isn't now which would leave some services running. Yes we probably should, but I think it should be done as a separate ticket. Simo. What's the impact if we don't do this now? some services will not be turned off by ipactl, however this shouldn't impact much at shutdown, as systemd will shutdown left over stuff as it keeps track of all processes. uhmm however if someone does a ipactl restart those services will not be restarted the first time it is run (we will tell systemd to start them but it will do likely nothing as they are already started), after that they will be handled as they fact they are enabled will be stored in the file. Well, what is the downside of a call to refresh the list of running services that should be run at the end of setting up new services? Otherwise we could find outselves with a bunch of unreproducible cases where things weren't start/stopped properly. Yeah I have been thinking about that. I am thinking of changing the patch so that it's the service class itself that saves this data. That way saving is always performed no matter 'what' starts the service. Do you agree ? Yes, that sounds like it'll be very robust. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 500 Fix shutdown issues with systemd
Simo Sorce wrote: Also improve shutdown reliability and restart behavior so we always kill all the processes we started even if the list of processes to handle changed in LDAP. Fixes: https://fedorahosted.org/freeipa/ticket/2302 Should this list be updated if we do a post-install of DNS or the CA? It isn't now which would leave some services running. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 500 Fix shutdown issues with systemd
Simo Sorce wrote: On Thu, 2012-10-18 at 11:37 -0400, Rob Crittenden wrote: Simo Sorce wrote: Also improve shutdown reliability and restart behavior so we always kill all the processes we started even if the list of processes to handle changed in LDAP. Fixes: https://fedorahosted.org/freeipa/ticket/2302 Should this list be updated if we do a post-install of DNS or the CA? It isn't now which would leave some services running. Yes we probably should, but I think it should be done as a separate ticket. Simo. What's the impact if we don't do this now? rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 500 Fix shutdown issues with systemd
On Thu, 2012-10-18 at 11:51 -0400, Rob Crittenden wrote: Simo Sorce wrote: On Thu, 2012-10-18 at 11:37 -0400, Rob Crittenden wrote: Simo Sorce wrote: Also improve shutdown reliability and restart behavior so we always kill all the processes we started even if the list of processes to handle changed in LDAP. Fixes: https://fedorahosted.org/freeipa/ticket/2302 Should this list be updated if we do a post-install of DNS or the CA? It isn't now which would leave some services running. Yes we probably should, but I think it should be done as a separate ticket. Simo. What's the impact if we don't do this now? some services will not be turned off by ipactl, however this shouldn't impact much at shutdown, as systemd will shutdown left over stuff as it keeps track of all processes. uhmm however if someone does a ipactl restart those services will not be restarted the first time it is run (we will tell systemd to start them but it will do likely nothing as they are already started), after that they will be handled as they fact they are enabled will be stored in the file. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 500 Fix shutdown issues with systemd
On Thu, 18 Oct 2012, Rob Crittenden wrote: Simo Sorce wrote: Also improve shutdown reliability and restart behavior so we always kill all the processes we started even if the list of processes to handle changed in LDAP. Fixes: https://fedorahosted.org/freeipa/ticket/2302 Should this list be updated if we do a post-install of DNS or the CA? It isn't now which would leave some services running. Same for ipa-adtrust-install. -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 500 Fix shutdown issues with systemd
Also improve shutdown reliability and restart behavior so we always kill
all the processes we started even if the list of processes to handle
changed in LDAP.
Fixes: https://fedorahosted.org/freeipa/ticket/2302
--
Simo Sorce * Red Hat, Inc * New York
From a4f2f7b8500a394d99c16692f1c586c0e906e7a4 Mon Sep 17 00:00:00 2001
From: Simo Sorce sso...@redhat.com
Date: Fri, 12 Oct 2012 15:58:02 -0400
Subject: [PATCH] Get list of service from LDAP only at startup
We dump the list retrieved from LDAP at startup in a temporary configuration
file and always use that file afterwards.
We check (possibly different) data from LDAP only at (re)start.
This way we always shutdown exactly the services we started even if the list
changed in the meanwhile (we avoid leaving a service running even if it was
removed from LDAP as the admin decided it should not be started in future).
This should also fix a problematic deadlock with systemd when we try to read
the list of service from LDAP at shutdown.
Simo.
---
freeipa.spec.in| 2 +
init/systemd/ipa.conf.tmpfiles | 1 +
install/tools/ipactl | 185 +++--
3 files changed, 144 insertions(+), 44 deletions(-)
diff --git a/freeipa.spec.in b/freeipa.spec.in
index 060f09b12e6891defc8cb00d4f52a0d019198a70..9393f81199d26afd69d04ee4cc2672fc83deb2c3 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -446,6 +446,7 @@ install -m 0644 init/systemd/ipa.conf.tmpfiles %{buildroot}%{_sysconfdir}/tmpfil
mkdir -p %{buildroot}%{_localstatedir}/run/
install -d -m 0700 %{buildroot}%{_localstatedir}/run/ipa_memcached/
+install -d -m 0700 %{buildroot}%{_localstatedir}/run/ipa/
mkdir -p %{buildroot}%{_libdir}/krb5/plugins/libkrb5
touch %{buildroot}%{_libdir}/krb5/plugins/libkrb5/winbind_krb5_locator.so
@@ -623,6 +624,7 @@ fi
%{_sysconfdir}/cron.d/ipa-compliance
%config(noreplace) %{_sysconfdir}/sysconfig/ipa_memcached
%dir %attr(0700,apache,apache) %{_localstatedir}/run/ipa_memcached/
+%dir %attr(0700,root,root) %{_localstatedir}/run/ipa/
%if 0%{?fedora} = 15
%config %{_sysconfdir}/tmpfiles.d/ipa.conf
%endif
diff --git a/init/systemd/ipa.conf.tmpfiles b/init/systemd/ipa.conf.tmpfiles
index e4b679a55d68a6b83991ac72dd520c32b2a0de50..1e7a896ed8df00c97f2d092504e2a65960bb341d 100644
--- a/init/systemd/ipa.conf.tmpfiles
+++ b/init/systemd/ipa.conf.tmpfiles
@@ -1 +1,2 @@
d /var/run/ipa_memcached 0700 apache apache
+d /var/run/ipa 0700 root root
diff --git a/install/tools/ipactl b/install/tools/ipactl
index d4b2c0878f2b62fd12198f76bef01ef70e9f3de1..39ccc346c08f18514ec8c5b90f59c0ff48e50757 100755
--- a/install/tools/ipactl
+++ b/install/tools/ipactl
@@ -34,6 +34,7 @@ try:
import ldap.sasl
import ldapurl
import socket
+import json
except ImportError:
print sys.stderr, \
There was a problem importing one of the required Python modules. The
@@ -44,6 +45,7 @@ error was:
sys.exit(1)
SASL_EXTERNAL = ldap.sasl.sasl({}, 'EXTERNAL')
+SVC_LIST_FILE = /var/run/ipa/services.list
class IpactlError(ScriptError):
pass
@@ -162,10 +164,32 @@ def get_config(dirsrv):
for p in entry[1]['ipaConfigString']:
if p.startswith('startOrder '):
order = p.split()[1]
-svc_list.append((order, name))
+svc_list.append([order, name])
return svc_list
+def get_config_from_file():
+
+svc_list = []
+
+try:
+f = open(SVC_LIST_FILE, 'r')
+svc_list = json.load(f)
+except Exception, e:
+raise IpactlError(Unknown error when retrieving list of services from file: + str(e))
+
+return svc_list
+
+def dump_config_to_file(svc_list):
+
+try:
+f = open(SVC_LIST_FILE, 'w')
+json.dump(svc_list, f)
+f.flush()
+f.close()
+except Exception, e:
+raise IpactlError(Unknown error when saving list of services to file: + str(e))
+
def ipa_start(options):
dirsrv = ipaservices.knownservices.dirsrv
try:
@@ -194,6 +218,8 @@ def ipa_start(options):
# no service to stop
return
+dump_config_to_file(svc_list)
+
for (order, svc) in sorted(svc_list):
svc_name = service.SERVICE_LIST[svc][0]
svchandle = ipaservices.service(svc_name)
@@ -220,11 +246,10 @@ def ipa_stop(options):
dirsrv = ipaservices.knownservices.dirsrv
svc_list = []
try:
-svc_list = get_config(dirsrv)
+svc_list = get_config_from_file()
except Exception, e:
-# ok if dirsrv died this may fail, so let's try to quickly restart it
-# and see if we can get anything. If not throw our hands up and just
-# exit
+# Issue reading the file ? Let's try to get data from LDAP as a
+# fallback
try:
dirsrv.start(capture_output=False)
svc_list = get_config(dirsrv)
@@ -259,55 +284,135 @@ def ipa_stop(options):
def ipa_restart(options):
dirsrv = ipaservices.knownservices.dirsrv
+
Re: [Freeipa-devel] [PATCH] 500 Fix shutdown issues with systemd
On Tue, 16 Oct 2012, Simo Sorce wrote: Also improve shutdown reliability and restart behavior so we always kill all the processes we started even if the list of processes to handle changed in LDAP. Fixes: https://fedorahosted.org/freeipa/ticket/2302 Works for me on freshly installed F18. Since I've got no signs of solution coming out of systemd maintainer, I'm inclined to ACK this patch. For uninitiated, the problem with systemd is that we attempt to start dirsrv services at the time when everything is going for shutdown. systemctl uses D-Bus for communication with systemd and at the time when we send start command for dirsrv services, there is no D-Bus daemon already so systemctl sits forever, waiting for any message on the bus. Avoiding restarting dirsrv services avoids coming into infinite loop territory, thus ACK. Security-wise, the dumped list of services is in the directory (0700, root, root) permissions so nobody can modify it but root. Root already has possibility to shutdown whatever services are there. I think we are OK here -- but whoever packages the change, would need to be careful and copy accompanying spec-file changes. -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
