[Freeipa-devel] [PATCH 0006] Improves sssd.conf handling during ipa-client uninstall
Martin Kosek
mkosek at redhat.com
Wed Sep 12 15:29:44 UTC 2012
On 08/29/2012 02:54 PM, Tomas Babej wrote:
> On 08/27/2012 04:55 PM, Martin Kosek wrote:
>> On 08/27/2012 03:37 PM, Jakub Hrozek wrote:
>>> On Mon, Aug 27, 2012 at 02:57:44PM +0200, Martin Kosek wrote:
>>>> I think that the right behavior of SSSD conf uninstall should be the
>>>> following:
>>>>
>>>> * sssd.conf existed before IPA install + non-IPA domains in sssd.conf found:
>>>> - move backed conf up sssd.conf.bkp (and inform the user)
>>>> - use SSSDConfig delete_domain function to remove ipa domain from sssd.conf
>>>> - restart sssd afterwards
>>> I'm confused here, which of the files is the original
>>> pre-ipa-client-install file?
>> This is the "backed up sssd.conf". I thought that it may be useful for user to
>> still have an access to it after uninstall.
>>
>>> How does the non-ipa domain end up in the sssd.conf file? Does it have
>>> to be configured manually or does ipa-client-install merge the list of
>>> domains on installation?
>> ipa-client-install merge the list of the domains. It overrides the old
>> sssd.conf only when we cannot parse the sssd.conf and --preserve-sssd option
>> was not set.
>>
>> Martin
> Hi,
>
> The sssd.conf file is no longer left behind in case sssd was not
> configured before the installation. However, the patch goes behind
> the scope of this ticked and improves the handling of sssd.conf
> during the ipa-client-install --uninstall in general.
>
> The current behaviour (well documented in source code) is as follows:
> - In general, the IPA domain is simply removed from the sssd.conf
> file, instead of sssd.conf being rewritten from the backup. This
> preserves any domains added after installation.
>
> - If sssd.conf existed before the installation, it is restored to
> sssd.conf.bkp. However, any IPA domains from pre-installation
> sssd.conf should have been merged during the installation.
>
> - If sssd.conf did not exist before the installation, and no other
> domains than IPA domain exist in it, the patch makes sure that
> sssd.conf is moved to sssd.conf.deleted so user experiences no
> crash during any next installation due to its existence.
>
> https://fedorahosted.org/freeipa/ticket/2740
>
> Tomas
>
Good job, SSSD uninstall process now looks more consistent and better
documented. I just found the following (mainly minor) issues. Comments in the
patch:
diff --git a/ipa-client/ipa-install/ipa-client-install
b/ipa-client/ipa-install/ipa-client-install
index
2e65921e8de2dfe68443f5b5875954d71dd48ed2..c5cef15e1fb3a3e1d7cfd070f4288d3839accfc8
100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -183,6 +183,36 @@ def nssldap_exists():
return (retval, files_found)
+# helper function for uninstall
+# deletes IPA domain from sssd.conf
+def delete_IPA_domain():
Function names should be lowercase -> delete_ipa_domain
+ sssd = ipaservices.service('sssd')
+ try:
+ sssdconfig = SSSDConfig.SSSDConfig()
+ sssdconfig.import_config()
+ domains = sssdconfig.list_active_domains()
+
+ IPA_domain_name = None
Variables should be lowercase -> ipa_domain_name
+
+ for name in domains:
+ domain = sssdconfig.get_domain(name)
+ try:
+ provider = domain.get_option('id_provider')
+ if provider == "ipa":
+ IPA_domain_name = name
+ break
+ except SSSDConfig.NoOptionError:
+ continue
+
+ if IPA_domain_name != None:
Do not use != with None, True, False - use "is not None".
+ sssdconfig.delete_domain(IPA_domain_name)
+ sssdconfig.write()
+ else:
+ root_logger.warning("IPA domain could not be found in " +
+ "sssd.conf and therefore not deleted")
+ except IOError:
+ root_logger.warning("IPA domain could not be deleted. No access to the
sssd.conf file.")
There should be full path to sssd.conf in this error message. It is very useful
sometimes.
+
def uninstall(options, env):
if not fstore.has_files():
@@ -212,7 +242,12 @@ def uninstall(options, env):
sssdconfig = SSSDConfig.SSSDConfig()
sssdconfig.import_config()
domains = sssdconfig.list_active_domains()
- if len(domains) > 1:
+ all_domains = sssdconfig.list_domains()
+
+ # we consider all the domains, because handling sssd.conf
+ # during uninstall is dependant on was_sssd_configured flag
+ # so the user does not lose info about inactive domains
+ if len(all_domains) > 1:
# There was more than IPA domain configured
was_sssd_configured = True
for name in domains:
@@ -349,6 +384,62 @@ def uninstall(options, env):
"Failed to remove krb5/LDAP configuration: %s", str(e))
return CLIENT_INSTALL_ERROR
+ # Next if-elif-elif construction deals with sssd.conf file.
+ # Old pre-IPA domains are preserved due merging the old sssd.conf
+ # during the installation of ipa-client but any new domains are
+ # only present in sssd.conf now, so we don't want to delete them
+ # by rewriting sssd.conf file. IPA domain is removed gracefully.
+
+ # SSSD was installed before our installation and other non-IPA domains
+ # found, restore backed up sssd.conf to sssd.conf.bkp and remove IPA
+ # domain from the current sssd.conf
+ if was_sssd_installed and was_sssd_configured:
+ root_logger.info(
+ "The original configuration of SSSD included other domains than " +
+ "the IPA-based one.")
+
+ delete_IPA_domain()
+
+ try:
+ os.rename("/etc/sssd/sssd.conf","/etc/sssd/sssd.conf.current")
+ if fstore.restore_file("/etc/sssd/sssd.conf"):
+ os.rename("/etc/sssd/sssd.conf","/etc/sssd/sssd.conf.bkp")
+ os.rename("/etc/sssd/sssd.conf.current","/etc/sssd/sssd.conf")
I don't like this very much. I would rather not mess with /etc/sssd/sssd.conf,
it may surprise other tools/user and we could also end with no
/etc/sssd/sssd.conf at all if OSError is raised in second or third step.
I would rather enhance fstore and implement function like "restore_file_to"
which would accept a second parameter with a custom path, like
"/etc/sssd/sssd.conf.bkp".
+ except OSError:
+ pass
The exception should be at least logged to debug log.
+
+ root_logger.info("Original pre-IPA SSSD configuration file was " +
+ "restored to /etc/sssd/sssd.conf.bkp.")
+ root_logger.info("IPA domain removed from current one, " +
+ "restarting SSSD service")
+ sssd = ipaservices.service('sssd')
+ try:
+ sssd.restart()
+ except CalledProcessError:
+ root_logger.warning("SSSD service restart was unsuccessful.")
+
+ # SSSD was not installed before our installation, but other domains found,
+ # delete IPA domain, but leave other domains intact
+ elif not was_sssd_installed and was_sssd_configured:
+ delete_IPA_domain()
+ root_logger.info("Other domains than IPA domain found, " +
+ "IPA domain was removed from sssd.conf.")
+ try:
+ sssd.restart()
+ except CalledProcessError:
+ root_logger.warning("SSSD service restart was unsuccessful.")
+
+ # SSSD was not installed before our installation, and no other domains
+ # than IPA are configured in sssd.conf - make sure config file is removed
+ elif not was_sssd_installed and not was_sssd_configured:
+ try:
+ os.rename("/etc/sssd/sssd.conf","/etc/sssd/sssd.conf.deleted")
+ except OSError:
+ pass
Error should be logged to debug log.
+
+ root_logger.info("Redundant SSSD configuration file " +
+ "/etc/sssd/sssd.conf was moved to /etc/sssd.conf.deleted")
+
if fstore.has_files():
root_logger.info("Restoring client configuration files")
fstore.restore_all_files()
@@ -428,20 +519,6 @@ def uninstall(options, env):
if was_sshd_configured and ipaservices.knownservices.sshd.is_running():
ipaservices.knownservices.sshd.restart()
- if was_sssd_installed and was_sssd_configured:
- # SSSD was installed before our installation, config now is restored,
restart it
- root_logger.info(
- "The original configuration of SSSD included other domains than " +
- "the IPA-based one.")
- root_logger.info(
- "Original configuration file was restored, restarting SSSD " +
- "service.")
- sssd = ipaservices.service('sssd')
- try:
- sssd.restart()
- except CalledProcessError:
- root_logger.warning("SSSD service restart was unsuccessful.")
-
if not options.unattended:
root_logger.info(
"The original nsswitch.conf configuration has been restored.")
--
1.7.11.4
Martin
More information about the Freeipa-devel
mailing list