On 17/02/15 12:18, David Kupka wrote:
On 02/11/2015 05:13 PM, Martin Basti wrote:
https://fedorahosted.org/freeipa/ticket/4869

Fixes:
- enable/start a service after uninstallation if the service was
enabled/running before in correct way
- store status of service before disable/stop/start/enable operation
- run uninstall only for configured services

Uninstall for ipa-dnskeysyncd is not executed because of
https://fedorahosted.org/freeipa/ticket/4901

Patches attached.



_______________________________________________
Freeipa-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/freeipa-devel

Hi,
thanks for patches. The code looks good to me but I found 2 issues:

1. httpd is left in non-working state after installing and uninstalling ipa-server. Tried on clean Fedora 21, the httpd was not configured before ipa-server installation.

2. Patch 191 needs (trivial) rebase.

Thanks,

Updated patches attached.

--
Martin Basti

From ab7c9a9762de4e15c91a07cccab881411ecc2f0e Mon Sep 17 00:00:00 2001
From: Martin Basti <[email protected]>
Date: Tue, 27 Jan 2015 11:04:03 +0100
Subject: [PATCH 1/4] Fix restoring services status during uninstall

Services hasn't been restored correctly, which causes disabling already
disabled services, or some service did not start. This patch fix these
issues.

Ticket: https://fedorahosted.org/freeipa/ticket/4869
---
 ipaserver/install/bindinstance.py        | 11 +++++------
 ipaserver/install/cainstance.py          |  6 ++++--
 ipaserver/install/dnskeysyncinstance.py  | 19 ++++++-------------
 ipaserver/install/dsinstance.py          |  5 +++--
 ipaserver/install/httpinstance.py        | 15 +++++++--------
 ipaserver/install/krbinstance.py         |  9 +++++----
 ipaserver/install/ntpinstance.py         | 14 ++++++++------
 ipaserver/install/odsexporterinstance.py | 14 ++++++--------
 ipaserver/install/opendnssecinstance.py  | 10 +++++-----
 ipaserver/install/service.py             | 15 +++++++++------
 10 files changed, 58 insertions(+), 60 deletions(-)

diff --git a/ipaserver/install/bindinstance.py b/ipaserver/install/bindinstance.py
index 4e630e8ddfed524d021d19016f48615fc8c0ab9d..cd1c7e7a67895808efdb29df963731edb2a2340e 100644
--- a/ipaserver/install/bindinstance.py
+++ b/ipaserver/install/bindinstance.py
@@ -1179,8 +1179,6 @@ class BindInstance(service.Service):
 
         self.dns_backup.clear_records(api.Backend.ldap2.isconnected())
 
-        if not running is None:
-            self.stop()
 
         for f in [NAMED_CONF, RESOLV_CONF]:
             try:
@@ -1189,11 +1187,12 @@ class BindInstance(service.Service):
                 root_logger.debug(error)
                 pass
 
-        if not enabled is None and not enabled:
-            self.disable()
+        # disabled by default, by ldap_enable()
+        if enabled:
+            self.enable()
 
-        if not running is None and running:
-            self.start()
+        if running:
+            self.restart()
 
         self.named_regular.unmask()
         if named_regular_enabled:
diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index a61534d50db0c811f314ff60fb6dc1825e70c137..8ba6e4616647746fce15a98f3e707cc04d349b0d 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -1273,8 +1273,10 @@ class CAInstance(DogtagInstance):
 
     def uninstall(self):
         enabled = self.restore_state("enabled")
-        if not enabled is None and not enabled:
-            self.disable()
+
+        # disabled by default, by ldap_enable()
+        if enabled:
+            self.enable()
 
         if self.dogtag_constants.DOGTAG_VERSION >= 10:
             DogtagInstance.uninstall(self)
diff --git a/ipaserver/install/dnskeysyncinstance.py b/ipaserver/install/dnskeysyncinstance.py
index 5da65d87b1471710b762f90b9a33c453c7d809b7..1396d01a7842623f30f863fd8a2330cab26fe5a2 100644
--- a/ipaserver/install/dnskeysyncinstance.py
+++ b/ipaserver/install/dnskeysyncinstance.py
@@ -124,8 +124,6 @@ class DNSKeySyncInstance(service.Service):
         self.fqdn = fqdn
         self.realm = realm_name
         self.suffix = ipautil.realm_to_suffix(self.realm)
-        self.backup_state("enabled", self.is_enabled())
-        self.backup_state("running", self.is_running())
         try:
             self.stop()
         except:
@@ -417,7 +415,6 @@ class DNSKeySyncInstance(service.Service):
                              self.suffix, self.extra_config)
         except errors.DuplicateEntry:
             self.logger.error("DNSKeySync service already exists")
-        self.enable()
 
     def __setup_principal(self):
         assert self.ods_gid is not None
@@ -480,11 +477,13 @@ class DNSKeySyncInstance(service.Service):
 
         self.print_msg("Unconfiguring %s" % self.service_name)
 
-        running = self.restore_state("running")
-        enabled = self.restore_state("enabled")
+        # Just eat states
+        self.restore_state("running")
+        self.restore_state("enabled")
 
-        if running is not None:
-            self.stop()
+        # stop and disable service (IPA service, we do not need it anymore)
+        self.stop()
+        self.disable()
 
         for f in [paths.SYSCONFIG_NAMED]:
             try:
@@ -500,9 +499,3 @@ class DNSKeySyncInstance(service.Service):
             os.remove(paths.DNSSEC_SOFTHSM_PIN)
         except Exception:
             pass
-
-        if enabled is not None and not enabled:
-            self.disable()
-
-        if running is not None and running:
-            self.start()
diff --git a/ipaserver/install/dsinstance.py b/ipaserver/install/dsinstance.py
index 1e07c6d0df2316f8238020cbb2cc403fac1b860f..6bf31da99f36aaa76bc5cd6b2c80f92f51f86698 100644
--- a/ipaserver/install/dsinstance.py
+++ b/ipaserver/install/dsinstance.py
@@ -771,8 +771,9 @@ class DsInstance(service.Service):
             root_logger.debug(error)
             pass
 
-        if not enabled is None and not enabled:
-            self.disable()
+        # disabled during IPA installation
+        if enabled:
+            self.enable()
 
         serverid = self.restore_state("serverid")
         if serverid is not None:
diff --git a/ipaserver/install/httpinstance.py b/ipaserver/install/httpinstance.py
index cda85ab02b8054748e671935fcfbc3993257c53e..18cf6bb1a55512f475bde62b2db7a775945a97ec 100644
--- a/ipaserver/install/httpinstance.py
+++ b/ipaserver/install/httpinstance.py
@@ -146,7 +146,7 @@ class HTTPInstance(service.Service):
         self.restart()
 
     def __enable(self):
-        self.backup_state("enabled", self.is_running())
+        self.backup_state("enabled", self.is_enabled())
         # We do not let the system start IPA components on its own,
         # Instead we reply on the IPA init script to start only enabled
         # components as found in our LDAP configuration tree
@@ -388,8 +388,6 @@ class HTTPInstance(service.Service):
         running = self.restore_state("running")
         enabled = self.restore_state("enabled")
 
-        if not running is None:
-            self.stop()
 
         self.stop_tracking_certificates()
 
@@ -407,9 +405,6 @@ class HTTPInstance(service.Service):
                 ca_iface.Set('org.fedorahosted.certmonger.ca',
                              'external-helper', helper)
 
-        if not enabled is None and not enabled:
-            self.disable()
-
         for f in [paths.HTTPD_IPA_CONF, paths.HTTPD_SSL_CONF, paths.HTTPD_NSS_CONF]:
             try:
                 self.fstore.restore_file(f)
@@ -430,8 +425,12 @@ class HTTPInstance(service.Service):
         except ipapython.errors.SetseboolError as e:
             self.print_msg('WARNING: ' + str(e))
 
-        if not running is None and running:
-            self.start()
+        if running:
+            self.restart()
+
+        # disabled by default, by ldap_enable()
+        if enabled:
+            self.enable()
 
     def stop_tracking_certificates(self):
         db = certs.CertDB(api.env.realm)
diff --git a/ipaserver/install/krbinstance.py b/ipaserver/install/krbinstance.py
index 6a480222fa4918a4950143f576138dc017cb58a7..266adb33b8acdea7a40c6ed74ffa95d7ddaa8155 100644
--- a/ipaserver/install/krbinstance.py
+++ b/ipaserver/install/krbinstance.py
@@ -454,11 +454,12 @@ class KrbInstance(service.Service):
                 root_logger.debug(error)
                 pass
 
-        if not enabled is None and not enabled:
-            self.disable()
+        # disabled by default, by ldap_enable()
+        if enabled:
+            self.enable()
 
-        if not running is None and running:
-            self.start()
+        if running:
+            self.restart()
 
         self.kpasswd = KpasswdInstance()
         self.kpasswd.uninstall()
diff --git a/ipaserver/install/ntpinstance.py b/ipaserver/install/ntpinstance.py
index f2edf9bc18ec5408d7c17a7f1d90235c1ac164a5..cdab0ae2629100cb91ef8f48716d41f2d9008475 100644
--- a/ipaserver/install/ntpinstance.py
+++ b/ipaserver/install/ntpinstance.py
@@ -165,8 +165,10 @@ class NTPInstance(service.Service):
         running = self.restore_state("running")
         enabled = self.restore_state("enabled")
 
-        if not running is None:
-            self.stop()
+        # service is not in LDAP, stop and disable service
+        # before restoring configuration
+        self.stop()
+        self.disable()
 
         try:
             self.fstore.restore_file(paths.NTP_CONF)
@@ -174,8 +176,8 @@ class NTPInstance(service.Service):
             root_logger.debug(error)
             pass
 
-        if not enabled is None and not enabled:
-            self.disable()
+        if enabled:
+            self.enable()
 
-        if not running is None and running:
-            self.start()
+        if running:
+            self.restart()
diff --git a/ipaserver/install/odsexporterinstance.py b/ipaserver/install/odsexporterinstance.py
index 57b1451c0566d57ef4208314cfadaf8e225a6958..e01550446dbefd276092ae8fa60c6b03f799610b 100644
--- a/ipaserver/install/odsexporterinstance.py
+++ b/ipaserver/install/odsexporterinstance.py
@@ -82,7 +82,6 @@ class ODSExporterInstance(service.Service):
                              self.suffix)
         except errors.DuplicateEntry:
             root_logger.error("DNSKeyExporter service already exists")
-        self.enable()
 
     def __setup_key_exporter(self):
         installutils.set_directive(paths.SYSOCNFIG_IPA_ODS_EXPORTER,
@@ -155,14 +154,13 @@ class ODSExporterInstance(service.Service):
 
         self.print_msg("Unconfiguring %s" % self.service_name)
 
-        running = self.restore_state("running")
-        enabled = self.restore_state("enabled")
+        # just eat states
+        self.restore_state("running")
+        self.restore_state("enabled")
 
-        if enabled is not None and not enabled:
-            self.disable()
-
-        if running is not None and running:
-            self.start()
+        # stop and disable service (IPA service, we do not need it anymore)
+        self.disable()
+        self.stop()
 
         # restore state of dnssec default signer daemon
         signerd_enabled = self.restore_state("singerd_enabled")
diff --git a/ipaserver/install/opendnssecinstance.py b/ipaserver/install/opendnssecinstance.py
index 0d2fb009ea7d0614b9fea573e85ed6913f24439c..869cf8ffec3196a3694442c75ce353211627cb18 100644
--- a/ipaserver/install/opendnssecinstance.py
+++ b/ipaserver/install/opendnssecinstance.py
@@ -149,7 +149,6 @@ class OpenDNSSECInstance(service.Service):
                              self.suffix, self.extra_config)
         except errors.DuplicateEntry:
             root_logger.error("DNSSEC service already exists")
-        self.enable()
 
     def __setup_conf_files(self):
         if not self.fstore.has_file(paths.OPENDNSSEC_CONF_FILE):
@@ -292,8 +291,9 @@ class OpenDNSSECInstance(service.Service):
                 root_logger.debug(error)
                 pass
 
-        if enabled is not None and not enabled:
-            self.disable()
+        # disabled by default, by ldap_enable()
+        if enabled:
+            self.enable()
 
-        if running is not None and running:
-            self.start()
+        if running:
+            self.restart()
diff --git a/ipaserver/install/service.py b/ipaserver/install/service.py
index 3ae43d8f39cae39389ea02a14f95953fd3888cd0..836d701a0b5f340fe3b2920341b278b0ff6e0fb2 100644
--- a/ipaserver/install/service.py
+++ b/ipaserver/install/service.py
@@ -518,11 +518,14 @@ class SimpleServiceInstance(Service):
         if self.is_configured():
             self.print_msg("Unconfiguring %s" % self.service_name)
 
+        self.stop()
+        self.disable()
+
         running = self.restore_state("running")
-        enabled = not self.restore_state("enabled")
+        enabled = self.restore_state("enabled")
 
-        if not running is None and not running:
-            self.stop()
-        if not enabled is None and not enabled:
-            self.disable()
-            self.remove()
+        # restore the original state of service
+        if running:
+            self.start()
+        if enabled:
+            self.enable()
-- 
2.1.0

From 99fabbb0e73c94093dffc81ffd3ca0939201deea Mon Sep 17 00:00:00 2001
From: Martin Basti <[email protected]>
Date: Tue, 27 Jan 2015 15:39:38 +0100
Subject: [PATCH 2/4] Fix do not enable service before storing status

Ticket: https://fedorahosted.org/freeipa/ticket/4869
---
 ipaserver/install/service.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/ipaserver/install/service.py b/ipaserver/install/service.py
index 836d701a0b5f340fe3b2920341b278b0ff6e0fb2..75285cd90ccef44e41296f379155e1ffbc74f735 100644
--- a/ipaserver/install/service.py
+++ b/ipaserver/install/service.py
@@ -506,7 +506,6 @@ class SimpleServiceInstance(Service):
         self.restart()
 
     def __enable(self):
-        self.enable()
         self.backup_state("enabled", self.is_enabled())
         if self.gensvc_name == None:
             self.enable()
-- 
2.1.0

From ac07d9bb0d4a79efc6deffe21ac4369c84bc4a16 Mon Sep 17 00:00:00 2001
From: Martin Basti <[email protected]>
Date: Mon, 9 Feb 2015 16:18:28 +0100
Subject: [PATCH 3/4] Uninstall configured services only

Fixes:
dnskeysyncisntance - requires a stored state to be uninstalled
bindinstance - uninstal service only if bind was configured by IPA

Ticket:https://fedorahosted.org/freeipa/ticket/4869
---
 install/tools/ipa-server-install        | 10 ++++++++--
 ipaserver/install/dnskeysyncinstance.py | 10 ++++++----
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install
index 8b2b8a3c2ffc150d0fb0385d2b71894cc6d421a6..56a43770d95387762bce09634bd1056ba7f20576 100755
--- a/install/tools/ipa-server-install
+++ b/install/tools/ipa-server-install
@@ -597,8 +597,14 @@ def uninstall():
     if ods_exporter.is_configured():
         ods_exporter.uninstall()
 
-    bindinstance.BindInstance(fstore).uninstall()
-    dnskeysyncinstance.DNSKeySyncInstance(fstore).uninstall()
+    bind = bindinstance.BindInstance(fstore)
+    if bind.is_configured():
+        bind.uninstall()
+
+    dnskeysync = dnskeysyncinstance.DNSKeySyncInstance(fstore)
+    if dnskeysync.is_configured():
+        dnskeysync.uninstall()
+
     httpinstance.HTTPInstance(fstore).uninstall()
     krbinstance.KrbInstance(fstore).uninstall()
     dsinstance.DsInstance(fstore=fstore).uninstall()
diff --git a/ipaserver/install/dnskeysyncinstance.py b/ipaserver/install/dnskeysyncinstance.py
index 1396d01a7842623f30f863fd8a2330cab26fe5a2..090c875052e09a6644e34b111ca6f25040968208 100644
--- a/ipaserver/install/dnskeysyncinstance.py
+++ b/ipaserver/install/dnskeysyncinstance.py
@@ -179,6 +179,9 @@ class DNSKeySyncInstance(service.Service):
         ):
             raise RuntimeError("DNS container does not exist")
 
+        # ready to be installed, storing a state is required to run uninstall
+        self.backup_state("configured", True)
+
     def __setup_dnssec_containers(self):
         """
         Setup LDAP containers for DNSSEC
@@ -472,14 +475,13 @@ class DNSKeySyncInstance(service.Service):
 
 
     def uninstall(self):
-        if not self.is_configured():
-            return
-
-        self.print_msg("Unconfiguring %s" % self.service_name)
+        if self.is_configured():
+            self.print_msg("Unconfiguring %s" % self.service_name)
 
         # Just eat states
         self.restore_state("running")
         self.restore_state("enabled")
+        self.restore_state("configured")
 
         # stop and disable service (IPA service, we do not need it anymore)
         self.stop()
-- 
2.1.0

From 0636b69c2d5a3a96ec34dfb30bbfa65fd57caa83 Mon Sep 17 00:00:00 2001
From: Martin Basti <[email protected]>
Date: Mon, 9 Feb 2015 17:28:45 +0100
Subject: [PATCH 4/4] Fix saving named restore status

Accidentaly status was stored after service was stopped by installer

Ticket: https://fedorahosted.org/freeipa/ticket/4869
---
 ipaserver/install/bindinstance.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/ipaserver/install/bindinstance.py b/ipaserver/install/bindinstance.py
index cd1c7e7a67895808efdb29df963731edb2a2340e..d487666735e47212ad08f7a5d40db290df2dbf5e 100644
--- a/ipaserver/install/bindinstance.py
+++ b/ipaserver/install/bindinstance.py
@@ -659,8 +659,6 @@ class BindInstance(service.Service):
             if self.get_state("running") is None:
                 # first time store status
                 self.backup_state("running", self.is_running())
-                self.backup_state("named-regular-running",
-                                  self.named_regular.is_running())
             self.restart()
         except Exception as e:
             root_logger.error("Named service failed to start (%s)", e)
@@ -682,6 +680,10 @@ class BindInstance(service.Service):
             root_logger.error("DNS service already exists")
 
         # disable named, we need to run named-pkcs11 only
+        if self.get_state("named-regular-running") is None:
+            # first time store status
+            self.backup_state("named-regular-running",
+                              self.named_regular.is_running())
         try:
             self.named_regular.stop()
         except Exception as e:
-- 
2.1.0

_______________________________________________
Freeipa-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to