[Freeipa-devel] [PATCH 71] improve handling of ds instances during uninstall

[Alexander Bokovoy]

On Wed, 04 Apr 2012, John Dennis wrote:
>Ticket #2502
>
>* remove the "running" flag from backup_state in cainstance.py and
>  dsinstance.py because it does not provide the correct
>  information. In cainstance the running flag was never referenced
>  because restarting dirsrv instances occurs later in dsinstance. In
>  dsinstance when the running flag is set it incorrectly identifed the
>  PKI ds instance configured earlier by cainstance. The intent was to
>  determine if there were any ds instances other than those owned by
>  IPA which will need to be restarted upon uninstall. Clearly the PKI
>  ds instance does not qualify. We were generating a traceback when at
>  the conclusion of dsinstance.uninstall we tried to start the
>  remaining ds instances as indicated by the running flag, but there
>  were none to restart (because the running flag had been set as a
>  consequence of the PKI ds instance).
>
>* We only want to restart ds instances if there are other ds instances
>  besides those owned by IPA. We shouldn't be stopping all ds
>  instances either, but that's going to be covered by another
>  ticket. The fix for restarting other ds instances at the end of
>  uninstall is to check and see if there are other ds instances
>  remaining after we've removed ours, if so we restart them. Also it's
>  irrelevant if those ds instances were not present when we installed,
>  it only matters if they exist after we restore things during
>  uninstall. If they are present we have to start them back up because
>  we shut them down during uninstall.
>
>* Add new function get_ds_instances() which returns a list of existing
>  ds instances.
>
>* fixed error messages that incorrectly stated it "failed to restart"
>  a ds instance when it should be "failed to create".
>---
>+def get_ds_instances():
>+    '''
>+    Return a sorted list of all 389ds instances.
>+
>+    If the instance name ends with '.removed' it is ignored. This
>+    matches 389ds behavior.
>+    '''
>+
>+    dirsrv_instance_dir='/etc/dirsrv'
>+    instance_prefix = 'slapd-'
>+
>+    instances = []
>+
>+    for basename in os.listdir(dirsrv_instance_dir):
>+        pathname = os.path.join(dirsrv_instance_dir, basename)
>+        # 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'):
>+                # Strip off prefix
>+                instance = basename[len(instance_prefix):]
>+                # Must be non-empty
>+                if instance:
>+                    instances.append(basename[len(instance_prefix):])
You have already generated basename[len(instance_prefix):], may be it
could be as simple as 
    instances.append(instance)
here?

-- 
/ Alexander Bokovoy