On 06/14/2016 09:25 AM, Stanislav Laznicka wrote:
On 06/13/2016 02:51 PM, Martin Babinsky wrote:
On 06/07/2016 10:14 AM, Stanislav Laznicka wrote:
https://fedorahosted.org/freeipa/ticket/5775



Umm, wouldn't it be better to augment the `Service.start()/restart()` methods themselves with parameters that will suppress exception raising and log an error instead of copy-pasting try: ... except: blocks?

A good point. SystemdService start()/restart() methods will need augmenting as well (signerd service) but I suppose that's about it for this issue. I'll send a patch updated accordingly.

Actually, adding an argument to SystemdService's start()/restart() methods turned out to be very, very bad idea for this purpose (it would end up with way worse copy-paste than the original patch). Attached is probably the most minimal solution.
From 33f9b79a68431b54e3a047f3f4a67dec5554803c Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka <slazn...@redhat.com>
Date: Tue, 14 Jun 2016 17:17:37 +0200
Subject: [PATCH] Uninstaller won't fail if service can't be started

https://fedorahosted.org/freeipa/ticket/5775
---
 ipaserver/install/bindinstance.py        |  2 +-
 ipaserver/install/httpinstance.py        |  2 +-
 ipaserver/install/krbinstance.py         |  2 +-
 ipaserver/install/ntpinstance.py         |  2 +-
 ipaserver/install/odsexporterinstance.py |  7 ++++++-
 ipaserver/install/opendnssecinstance.py  |  2 +-
 ipaserver/install/service.py             | 28 +++++++++++++++++++++++-----
 7 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/ipaserver/install/bindinstance.py b/ipaserver/install/bindinstance.py
index 78e75359266bbefe7954242b98922272fb0c9194..b9b9a7d7f3bef5fb461182e23a4b45d39a3f0405 100644
--- a/ipaserver/install/bindinstance.py
+++ b/ipaserver/install/bindinstance.py
@@ -1273,7 +1273,7 @@ class BindInstance(service.Service):
             self.enable()
 
         if running:
-            self.restart()
+            self.restart(suppress_errors=True)
 
         self.named_regular.unmask()
         if named_regular_enabled:
diff --git a/ipaserver/install/httpinstance.py b/ipaserver/install/httpinstance.py
index 00f890175ae583f485797da6f913a7f83b302df3..cd81bbed3830e239b6cf50f765f176547ee788fa 100644
--- a/ipaserver/install/httpinstance.py
+++ b/ipaserver/install/httpinstance.py
@@ -549,7 +549,7 @@ class HTTPInstance(service.Service):
             self.print_msg('WARNING: ' + str(e))
 
         if running:
-            self.restart()
+            self.restart(suppress_errors=True)
 
         # disabled by default, by ldap_enable()
         if enabled:
diff --git a/ipaserver/install/krbinstance.py b/ipaserver/install/krbinstance.py
index f560a6ec4c2e4ce931cc1552976db5900a3fa5cd..61d02f9fd346d7b78e5d8682038d9b1a1a149c74 100644
--- a/ipaserver/install/krbinstance.py
+++ b/ipaserver/install/krbinstance.py
@@ -405,7 +405,7 @@ class KrbInstance(service.Service):
             self.enable()
 
         if running:
-            self.restart()
+            self.restart(suppress_errors=True)
 
         self.kpasswd = KpasswdInstance()
         self.kpasswd.uninstall()
diff --git a/ipaserver/install/ntpinstance.py b/ipaserver/install/ntpinstance.py
index 8b0f0e5395dae3c058fc31bd8914741e4d158830..8b2b1c1ec5102355eb6461bcfa5b9985ba02bc12 100644
--- a/ipaserver/install/ntpinstance.py
+++ b/ipaserver/install/ntpinstance.py
@@ -183,4 +183,4 @@ class NTPInstance(service.Service):
             self.enable()
 
         if running:
-            self.restart()
+            self.restart(suppress_errors=True)
diff --git a/ipaserver/install/odsexporterinstance.py b/ipaserver/install/odsexporterinstance.py
index e9f7bf853d98237aa19aace384b8ff7021c3a85a..4625df81988ba654ac844f93812fc6802b48dee8 100644
--- a/ipaserver/install/odsexporterinstance.py
+++ b/ipaserver/install/odsexporterinstance.py
@@ -193,7 +193,12 @@ class ODSExporterInstance(service.Service):
             signerd_service.enable()
 
         if signerd_running:
-            signerd_service.start()
+            try:
+                signerd_service.start()
+            except Exception as e:
+                root_logger.error("Unable to start '{svcname}': {err}"
+                                  .format(svcname=signerd_service.service_name,
+                                          err=e))
 
         installutils.remove_keytab(paths.IPA_ODS_EXPORTER_KEYTAB)
         installutils.remove_ccache(ccache_path=paths.IPA_ODS_EXPORTER_CCACHE)
diff --git a/ipaserver/install/opendnssecinstance.py b/ipaserver/install/opendnssecinstance.py
index f0c512ba04129d08b5874f58c7a25620f7435b2a..c4de0eb4614c235981840db0041a62f4db42ccef 100644
--- a/ipaserver/install/opendnssecinstance.py
+++ b/ipaserver/install/opendnssecinstance.py
@@ -386,4 +386,4 @@ class OpenDNSSECInstance(service.Service):
             self.enable()
 
         if running:
-            self.restart()
+            self.restart(suppress_errors=True)
diff --git a/ipaserver/install/service.py b/ipaserver/install/service.py
index 40767acd57d5e1fa8126144ca64f6951848ce214..1fcc190a8a5b939c44925fc2f10a7ef676655f17 100644
--- a/ipaserver/install/service.py
+++ b/ipaserver/install/service.py
@@ -340,11 +340,29 @@ class Service(object):
     def stop(self, instance_name="", capture_output=True):
         self.service.stop(instance_name, capture_output=capture_output)
 
-    def start(self, instance_name="", capture_output=True, wait=True):
-        self.service.start(instance_name, capture_output=capture_output, wait=wait)
+    def start(self, instance_name="", capture_output=True, wait=True,
+              suppress_errors=False):
+        try:
+            self.service.start(instance_name, capture_output=capture_output,
+                               wait=wait)
+        except Exception as e:
+            if suppress_errors:
+                root_logger.error("Unable to start '{svcname}': {err}"
+                                  .format(svcname=self.service_name, err=e))
+            else:
+                raise
 
-    def restart(self, instance_name="", capture_output=True, wait=True):
-        self.service.restart(instance_name, capture_output=capture_output, wait=wait)
+    def restart(self, instance_name="", capture_output=True, wait=True,
+                suppress_errors=False):
+        try:
+            self.service.restart(instance_name, capture_output=capture_output,
+                                 wait=wait)
+        except Exception as e:
+            if suppress_errors:
+                root_logger.error("Unable to restart '{svcname}': {err}"
+                                  .format(svcname=self.service_name, err=e))
+            else:
+                raise
 
     def is_running(self):
         return self.service.is_running()
@@ -604,6 +622,6 @@ class SimpleServiceInstance(Service):
 
         # restore the original state of service
         if running:
-            self.start()
+            self.start(suppress_errors=True)
         if enabled:
             self.enable()
-- 
2.5.5

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to