On 01/23/2015 05:56 PM, Martin Basti wrote:
On 22/01/15 15:03, Martin Babinsky wrote:
On 01/22/2015 12:38 PM, Martin Babinsky wrote:
On 01/22/2015 12:19 PM, Martin Kosek wrote:
On 01/22/2015 11:58 AM, Martin Babinsky wrote:
On 01/22/2015 11:04 AM, Martin Babinsky wrote:
The attached patch addresses
https://fedorahosted.org/freeipa/ticket/4487.

Using 'remove-ds.pl' script from 389-ds during server/replica
uninstallation should allow for cleaner removal of DS instance
with no
leftovers (opened ports etc).

Martin^3


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

Thanks to Martin^2 for explaining the rules governing the placement
of new
attributes into BasePathNamespace class.

Attaching updated patch.

Martin^3

I see you kept erase_ds_instance_data still in the dsinstance. What is
the
motivation? I thought that just like with PKI, with DS we will also
have
uninstall based solely only on the DS uninstall script and remove our
custom
removal.

We can keep the manual removal somewhere in wiki, but I would
personally not
keep/maintain it in FreeIPA code.

I originally thought that I will keep erase_ds_instance-data as a
failsafe method to clean up the dirsrv data in th case that remove-ds.pl
fails.

But I will remove the method altogether and change the code so that it
prints out some warning about manual removal of DS data when
remove-ds.pl fails.

Martin^2

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

Martin^3


Hi,

thank you for patch, but I have a few nitpicks:

1) Extra parenthesis
+                root_logger.error(("Failed to remove CA DS instance.
You may "
+                                   "need to remove instance data
manually"))

and (twice)

+                root_logger.error(("Failed to remove DS instance. You
may "
+                                   "need to remove instance data
manually"))

and

+        root_logger.debug(("'%s' failed. "
+                          "Attempting to force removal" %
paths.REMOVE_DS_PL))

2) Remove unused paths:
USR_LIB_DIRSRV_SLAPD_INSTANCE_DIR_TEMPLATE
SLAPD_INSTANCE_LOCK_TEMPLATE
USR_LIB_SLAPD_INSTANCE_TEMPLATE

Please do double check.

3) IMO we should avoid hardcoded value 'slapd'
instance_name = '-'.join(['slapd', serverid])

you may create module variable DS_INSTANCE_PREFIX and use it. Dont
forget to change it in following place as well.
ipaserver/install/dsinstance.py:117:    instance_prefix = 'slapd-'

4)
DS keytab hasn't been removed, it is okay?
It is created by IPA (krbinstance), not by DS install script.


Martin^2

Thanks for suggestions, attaching updated patch

Martin^3
From 41d3b8fc03cd57070a8173f5eb489fab9cf76fe4 Mon Sep 17 00:00:00 2001
From: Martin Babinsky <mbabi...@redhat.com>
Date: Wed, 21 Jan 2015 13:40:36 +0100
Subject: [PATCH] Use 'remove-ds.pl' to remove DS instance

The patch adds a function which calls 'remove-ds.pl' during DS instance
removal. This should allow for a more thorough removal of DS related data
during server uninstallation (such as closing custom ports, cleaning up
slapd-* entries etc.)

This patch is related to https://fedorahosted.org/freeipa/ticket/4487.
---
 install/tools/ipa-server-install |  6 +++-
 ipaplatform/base/paths.py        |  4 +--
 ipaserver/install/cainstance.py  |  8 ++++--
 ipaserver/install/dsinstance.py  | 62 +++++++++++++++++++++++-----------------
 4 files changed, 47 insertions(+), 33 deletions(-)

diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install
index 11055aee350d258b10a2efc97fd8b184909eae10..d78e8512f43cf6d0d12c761fa4dc6b13a0ba9aaf 100755
--- a/install/tools/ipa-server-install
+++ b/install/tools/ipa-server-install
@@ -414,7 +414,11 @@ def signal_handler(signum, frame):
         print "Removing configuration for %s instance" % ds.serverid
         ds.stop()
         if ds.serverid:
-            dsinstance.erase_ds_instance_data (ds.serverid)
+            try:
+                dsinstance.remove_ds_instance(ds.serverid)
+            except ipautil.CalledProcessError:
+                root_logger.error(("Failed to remove DS instance. You may "
+                                   "need to remove instance data manually"))
     sys.exit(1)
 
 def read_cache(dm_password):
diff --git a/ipaplatform/base/paths.py b/ipaplatform/base/paths.py
index 5c52714abaf7c5bddbeb80c68bd7cd6e0cac4459..7922e3bbc7b74fdc94eaa555d02f6fd6a5db5048 100644
--- a/ipaplatform/base/paths.py
+++ b/ipaplatform/base/paths.py
@@ -193,14 +193,12 @@ class BasePathNamespace(object):
     BIND_LDAP_DNS_IPA_WORKDIR = "/var/named/dyndb-ldap/ipa/"
     BIND_LDAP_DNS_ZONE_WORKDIR = "/var/named/dyndb-ldap/ipa/master/"
     USR_LIB_DIRSRV = "/usr/lib/dirsrv"
-    USR_LIB_SLAPD_INSTANCE_TEMPLATE = "/usr/lib/dirsrv/slapd-%s"
     USR_LIB_SLAPD_PKI_IPA_DIR = "/usr/lib/dirsrv/slapd-PKI-IPA"
     LIB_FIREFOX = "/usr/lib/firefox"
     LIBSOFTHSM2_SO = "/usr/lib/pkcs11/libsofthsm2.so"
     LIB_SYSTEMD_SYSTEMD_DIR = "/usr/lib/systemd/system/"
     BIND_LDAP_SO_64 = "/usr/lib64/bind/ldap.so"
     USR_LIB_DIRSRV_64 = "/usr/lib64/dirsrv"
-    USR_LIB_DIRSRV_SLAPD_INSTANCE_DIR_TEMPLATE = "/usr/lib64/dirsrv/slapd-%s"
     SLAPD_PKI_IPA = "/usr/lib64/dirsrv/slapd-PKI-IPA"
     LIB64_FIREFOX = "/usr/lib64/firefox"
     LIBSOFTHSM2_SO_64 = "/usr/lib64/pkcs11/libsofthsm2.so"
@@ -224,6 +222,7 @@ class BasePathNamespace(object):
     NTPD = "/usr/sbin/ntpd"
     PKIDESTROY = "/usr/sbin/pkidestroy"
     PKISPAWN = "/usr/sbin/pkispawn"
+    REMOVE_DS_PL = "/usr/sbin/remove-ds.pl"
     RESTORECON = "/usr/sbin/restorecon"
     SELINUXENABLED = "/usr/sbin/selinuxenabled"
     SETSEBOOL = "/usr/sbin/setsebool"
@@ -293,7 +292,6 @@ class BasePathNamespace(object):
     SSSD_PUBCONF_KNOWN_HOSTS = "/var/lib/sss/pubconf/known_hosts"
     SSSD_PUBCONF_KRB5_INCLUDE_D_DIR = "/var/lib/sss/pubconf/krb5.include.d/"
     DIRSRV_LOCK_DIR = "/var/lock/dirsrv"
-    SLAPD_INSTANCE_LOCK_TEMPLATE = "/var/lock/dirsrv/slapd-%s"
     VAR_LOG_DIRSRV_INSTANCE_TEMPLATE = "/var/log/dirsrv/slapd-%s"
     SLAPD_INSTANCE_ACCESS_LOG_TEMPLATE = "/var/log/dirsrv/slapd-%s/access"
     SLAPD_INSTANCE_ERROR_LOG_TEMPLATE = "/var/log/dirsrv/slapd-%s/errors"
diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index 2c6933be12bc02877886823c58c506bc6cbf5ed8..a61534d50db0c811f314ff60fb6dc1825e70c137 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -308,13 +308,17 @@ class CADSInstance(service.Service):
         if not enabled is None and not enabled:
             services.knownservices.dirsrv.disable()
 
-        if not serverid is None:
+        if serverid is not None:
             # drop the trailing / off the config_dirname so the directory
             # will match what is in certmonger
             dirname = dsinstance.config_dirname(serverid)[:-1]
             dsdb = certs.CertDB(self.realm, nssdir=dirname)
             dsdb.untrack_server_cert("Server-Cert")
-            dsinstance.erase_ds_instance_data(serverid)
+            try:
+                dsinstance.remove_ds_instance(serverid)
+            except ipautil.CalledProcessError:
+                root_logger.error("Failed to remove CA DS instance. You may "
+                                  "need to remove instance data manually")
 
         self.restore_state("user_exists")
 
diff --git a/ipaserver/install/dsinstance.py b/ipaserver/install/dsinstance.py
index 66267f4cdde548266b15594e3640bf8247c64859..1e07c6d0df2316f8238020cbb2cc403fac1b860f 100644
--- a/ipaserver/install/dsinstance.py
+++ b/ipaserver/install/dsinstance.py
@@ -64,6 +64,7 @@ IPA_SCHEMA_FILES = ("60kerberos.ldif",
                     "15rfc4876.ldif")
 
 ALL_SCHEMA_FILES = IPA_SCHEMA_FILES + ("05rfc2247.ldif", )
+DS_INSTANCE_PREFIX = 'slapd-'
 
 
 def find_server_root():
@@ -81,29 +82,29 @@ def config_dirname(serverid):
 def schema_dirname(serverid):
     return config_dirname(serverid) + "/schema/"
 
-def erase_ds_instance_data(serverid):
-    installutils.rmtree(paths.ETC_DIRSRV_SLAPD_INSTANCE_TEMPLATE % serverid)
 
-    installutils.rmtree(paths.USR_LIB_SLAPD_INSTANCE_TEMPLATE % serverid)
+def remove_ds_instance(serverid, force=False):
+    """A wrapper around the 'remove-ds.pl' script used by
+    389ds to remove a single directory server instance. In case of error
+    additional call with the '-f' flag is performed (forced removal). If this
+    also fails, then an exception is raised.
+    """
+    instance_name = ''.join([DS_INSTANCE_PREFIX, serverid])
+    args = [paths.REMOVE_DS_PL, '-i', instance_name]
+    if force:
+        args.append('-f')
+        root_logger.debug("Forcing instance removal")
+
+    try:
+        ipautil.run(args)
+    except ipautil.CalledProcessError:
+        if force:
+            root_logger.error("Instance removal failed.")
+            raise
+        root_logger.debug("'%s' failed. "
+                          "Attempting to force removal" % paths.REMOVE_DS_PL)
+        remove_ds_instance(serverid, force=True)
 
-    installutils.rmtree(paths.USR_LIB_DIRSRV_SLAPD_INSTANCE_DIR_TEMPLATE % serverid)
-
-    installutils.rmtree(paths.VAR_LIB_SLAPD_INSTANCE_DIR_TEMPLATE % serverid)
-
-    installutils.rmtree(paths.SLAPD_INSTANCE_LOCK_TEMPLATE % serverid)
-
-    installutils.remove_file(paths.SLAPD_INSTANCE_SOCKET_TEMPLATE % serverid)
-
-    installutils.rmtree(paths.VAR_LIB_DIRSRV_INSTANCE_SCRIPTS_TEMPLATE % serverid)
-
-    installutils.remove_file(paths.DS_KEYTAB)
-
-    installutils.remove_file(paths.SYSCONFIG_DIRSRV_INSTANCE % serverid)
-
-#    try:
-#        shutil.rmtree(paths.VAR_LOG_DIRSRV_INSTANCE_TEMPLATE % serverid)
-#    except:
-#        pass
 
 def get_ds_instances():
     '''
@@ -113,8 +114,7 @@ def get_ds_instances():
     matches 389ds behavior.
     '''
 
-    dirsrv_instance_dir=paths.ETC_DIRSRV
-    instance_prefix = 'slapd-'
+    dirsrv_instance_dir = paths.ETC_DIRSRV
 
     instances = []
 
@@ -123,9 +123,10 @@ def get_ds_instances():
         # Must be a directory
         if os.path.isdir(pathname):
             # Must start with prefix and not end with .removed
-            if basename.startswith(instance_prefix) and not basename.endswith('.removed'):
+            if (basename.startswith(DS_INSTANCE_PREFIX) and
+                    not basename.endswith('.removed')):
                 # Strip off prefix
-                instance = basename[len(instance_prefix):]
+                instance = basename[len(DS_INSTANCE_PREFIX):]
                 # Must be non-empty
                 if instance:
                     instances.append(instance)
@@ -774,9 +775,16 @@ class DsInstance(service.Service):
             self.disable()
 
         serverid = self.restore_state("serverid")
-        if not serverid is None:
+        if serverid is not None:
             self.stop_tracking_certificates(serverid)
-            erase_ds_instance_data(serverid)
+            root_logger.debug("Removing DS instance %s" % serverid)
+            try:
+                remove_ds_instance(serverid)
+                root_logger.debug("Removing DS keytab")
+                installutils.remove_file(paths.DS_KEYTAB)
+            except ipautil.CalledProcessError:
+                root_logger.error("Failed to remove DS instance. You may "
+                                  "need to remove instance data manually")
 
         # At one time we removed this user on uninstall. That can potentially
         # orphan files, or worse, if another useradd runs in the intermim,
-- 
2.1.0

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

Reply via email to