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