On 07/08/2013 08:32 AM, Alexander Bokovoy wrote:
> On Thu, 20 Jun 2013, Ana Krivokapic wrote:
>> Hello,
>>
>> Attached patches fix systemd and ipactl related bugs:
>>
>> https://fedorahosted.org/freeipa/ticket/3730
>> https://fedorahosted.org/freeipa/ticket/3729
> NACK. For me upgrade case fails (rpm -Uhv), dirsrv didn't restart on
> upgrade properly and everything else has failed afterwards.
>

This was caused due to 'systemctl is-active' returning exit status 3
('activating'), and our code treating the non-zero exit status as a failure. I
handled this case in the updated patch.

As for the ipa.service and dependency ordering, I have done some further testing
and found out the adding the '--ignore-dependencies' switch alone solves the
shutdown issue. So I think that no modification of ipa.service file is 
necessary.

Updated patches are attached.

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

From c8d6d367f686d9477f66984e6b18de419cb28242 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic <akriv...@redhat.com>
Date: Thu, 20 Jun 2013 11:53:29 +0200
Subject: [PATCH] Use correct DS instance in ipactl status

Make sure ipactl status check for correct DS instance. It should check for
'dirsrv@IPA-REALM' and not 'dirsrv.target'.

https://fedorahosted.org/freeipa/ticket/3730
---
 ipapython/ipautil.py               |  5 +++--
 ipapython/platform/base/systemd.py | 39 +++++++++++++++++++++++++++-----------
 2 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index 3d174ed021bdad7fef52061ecbb2470ee02ebe19..f2ca9d6a907a4a5fddc8e79b8c07a9d52c1b370e 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -58,9 +58,10 @@ class CalledProcessError(Exception):
         """This exception is raised when a process run by check_call() returns
         a non-zero exit status. The exit status will be stored in the
         returncode attribute."""
-        def __init__(self, returncode, cmd):
+        def __init__(self, returncode, cmd, output=None):
             self.returncode = returncode
             self.cmd = cmd
+            self.output = output
         def __str__(self):
             return "Command '%s' returned non-zero exit status %d" % (self.cmd, self.returncode)
 
@@ -319,7 +320,7 @@ def run(args, stdin=None, raiseonerr=True,
         root_logger.debug('stderr=%s' % stderr)
 
     if p.returncode != 0 and raiseonerr:
-        raise CalledProcessError(p.returncode, arg_string)
+        raise CalledProcessError(p.returncode, arg_string, stdout)
 
     return (stdout, stderr, p.returncode)
 
diff --git a/ipapython/platform/base/systemd.py b/ipapython/platform/base/systemd.py
index a9c1ec03261ae8694b1a308b2e86f342f27e9e62..9721ac1a92077048d8dd0248014ed7cb11bb61a8 100644
--- a/ipapython/platform/base/systemd.py
+++ b/ipapython/platform/base/systemd.py
@@ -18,8 +18,6 @@
 #
 
 import os
-import shutil
-import sys
 
 from ipapython import ipautil
 from ipapython.platform import base
@@ -36,12 +34,18 @@ def __init__(self, service_name, systemd_name):
         self.lib_path = os.path.join(self.SYSTEMD_LIB_PATH, self.systemd_name)
         self.lib_path_exists = None
 
-    def service_instance(self, instance_name):
+    def service_instance(self, instance_name, operation=None):
         if self.lib_path_exists is None:
             self.lib_path_exists = os.path.exists(self.lib_path)
 
         elements = self.systemd_name.split("@")
 
+        # Make sure the correct DS instance is returned
+        if (elements[0] == 'dirsrv' and
+                not instance_name and
+                operation == 'is-active'):
+            return 'dirsrv@%s.service' % str(api.env.realm.replace('.', '-'))
+
         # Short-cut: if there is already exact service name, return it
         if self.lib_path_exists and len(instance_name) == 0:
             if len(elements) == 1:
@@ -118,14 +122,27 @@ def restart(self, instance_name="", capture_output=True, wait=True):
             self.__wait_for_open_ports(self.service_instance(instance_name))
 
     def is_running(self, instance_name=""):
-        ret = True
-        try:
-            (sout, serr, rcode) = ipautil.run(["/bin/systemctl", "is-active", self.service_instance(instance_name)],capture_output=True)
-            if rcode != 0:
-                ret = False
-        except ipautil.CalledProcessError:
-                ret = False
-        return ret
+        instance = self.service_instance(instance_name, 'is-active')
+
+        while True:
+            try:
+                (sout, serr, rcode) = ipautil.run(
+                    ["/bin/systemctl", "is-active", instance],
+                    capture_output=True
+                )
+            except ipautil.CalledProcessError as e:
+                if e.returncode == 3 and 'activating' in str(e.output):
+                    continue
+                return False
+            else:
+                # activating
+                if rcode == 3 and 'activating' in str(sout):
+                    continue
+                # active
+                if rcode == 0:
+                    return True
+                # not active
+                return False
 
     def is_installed(self):
         installed = True
-- 
1.8.1.4

From f94a8bc3867b2a7a2dfb870e5ffbc2e41b944cfb Mon Sep 17 00:00:00 2001
From: Ana Krivokapic <akriv...@redhat.com>
Date: Thu, 20 Jun 2013 17:04:49 +0200
Subject: [PATCH] Avoid systemd service deadlock during shutdown

https://fedorahosted.org/freeipa/ticket/3729
---
 ipapython/platform/base/systemd.py | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/ipapython/platform/base/systemd.py b/ipapython/platform/base/systemd.py
index 57355e914c08a9fa4102707bb6d57cd33b8ff1f5..3bcbfa3c2bba99cb3f0d322ffe7ae4de65d9fec1 100644
--- a/ipapython/platform/base/systemd.py
+++ b/ipapython/platform/base/systemd.py
@@ -97,7 +97,17 @@ class SystemdService(base.PlatformService):
             ipautil.wait_for_open_ports('localhost', ports, api.env.startup_timeout)
 
     def stop(self, instance_name="", capture_output=True):
-        ipautil.run(["/bin/systemctl", "stop", self.service_instance(instance_name)], capture_output=capture_output)
+        instance = self.service_instance(instance_name)
+
+        # The --ignore-dependencies switch is used to avoid possible
+        # deadlock during the shutdown transaction. For more details, see
+        # https://fedorahosted.org/freeipa/ticket/3729#comment:1 and
+        # https://bugzilla.redhat.com/show_bug.cgi?id=973331#c11
+        ipautil.run(
+            ["/bin/systemctl", "stop", instance, "--ignore-dependencies"],
+            capture_output=capture_output
+        )
+
         if 'context' in api.env and api.env.context in ['ipactl', 'installer']:
             update_service_list = True
         else:
-- 
1.8.3.1

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

Reply via email to