[SSSD] [PATCH] CONFDB: Assume config file version 2 if missing

Jakub Hrozek jhrozek at redhat.com
Tue Sep 1 09:11:58 UTC 2015


On Tue, Sep 01, 2015 at 11:00:15AM +0200, Lukas Slebodnik wrote:
> On (01/09/15 10:51), Jakub Hrozek wrote:
> >On Tue, Aug 11, 2015 at 07:47:00AM +0200, Lukas Slebodnik wrote:
> >> On (10/08/15 17:18), Michal Židek wrote:
> >> >On 08/06/2015 01:39 PM, Lukas Slebodnik wrote:
> >> >>On (05/08/15 13:41), Michal Židek wrote:
> >> >>>On 07/30/2015 06:08 PM, Lukas Slebodnik wrote:
> >> >>>>>>>-    } else if (version < CONFDB_VERSION_INT) {
> >> >>>>>>>-        DEBUG(SSSDBG_FATAL_FAILURE,
> >> >>>>>>>-                "Config file is an old version. "
> >> >>>>>>>-                 "Please run configuration upgrade script.\n");
> >> >>>>>>>-        ret = EINVAL;
> >> >>>>>>>-        goto done;
> >> >>>>>>>-    } else if (version > CONFDB_VERSION_INT) {
> >> >>>>>>>-        DEBUG(SSSDBG_FATAL_FAILURE,
> >> >>>>>>>-                "Config file version is newer than confdb\n");
> >> >>>>>>>-        ret = EINVAL;
> >> >>>>>>>-        goto done;
> >> >>>>>>>+        /* No known version. Use default. */
> >> >>>>>>>+        DEBUG(SSSDBG_CONF_SETTINGS,
> >> >>>>>>>+              "Value of config_file_version option not found. "
> >> >>>>>>>+              "Assumed to be version %d.\n", CONFDB_DEFAULT_CFG_FILE_VER);
> >> >>>>>>>+    } else {
> >> >>>>>>>+        version = sss_ini_get_int_config_value(init_data,
> >> >>>>>>>+                                               CONFDB_DEFAULT_CFG_FILE_VER,
> >> >>>>>>>+                                               -1, &ret);
> >> >>>>>>>+        if (ret != EOK) {
> >> >>>>>>>+            DEBUG(SSSDBG_FATAL_FAILURE,
> >> >>>>>>>+                    "Config file version could not be determined\n");
> >> >>>>                       ^^
> >> >>>>I do not prefer nested "if"s. If you decided to do it in this way then
> >> >>>
> >> >>>Me neither, but sss_ini_get_int_config_value() has to be
> >> >>>skipped conditionally. It is just call to the function
> >> >>>plus error checking that is nested. I think it is not
> >> >>>too bad in this case.
> >> >>>
> >> >>>>you shoudl have proper indentation.
> >> >>>>
> >> >>>
> >> >>>Fixed in the new version.
> >> >>>
> >> >>It works because integration tests passed.
> >> >>http://sssd-ci.duckdns.org/logs/job/20/68/summary.html
> >> >>But ...
> >> >>
> >> >>I tested new version with ipa-client-install
> >> >>and "config_file_version = 2" is still added to sssd.conf
> >> >>even though it is a default value.
> >> >>
> >> >>ipa-client-install uses our python API (python-sssdconfig)
> >> >>and it does not try to add this option itself.
> >> >
> >> >I often change version of SSSD with git checkout sssd<version>.
> >> >If I generate the config with realmd or ipa-client-install
> >> realmd do not use python module SSSDConfig.
> >> 
> >> >with latest version then the config_version_file
> >> >would need to be added manually (and I am pretty
> >> >sure it would be after I looked into logs to see why sssd
> >> >is not starting). I know this will probably only hit
> >> >testers/developers, but I would prefer not to add unnecessary
> >> just developers and developers useually join machine
> >> to AD or IPA just once.
> >> 
> >> >little inconveniences.
> >> >
> >> I do not try old sssd version very often. Just in case of bisect.
> >> and ther is nice/simple workarount for "little inconvenience"
> >> 
> >> Just manually add "config_version_file = 2" to sssd.conf
> >> 
> >> The main point of this ticket is to simplify sssd.conf
> >> and our python sssdconfig API should do the same.
> >> Otherwise we do not need to do such change.
> >> 
> >> I still see "config_file_version = 2" in sssd.conf
> >> It was generated by python module SSSDConfig (via ipa-client-install)
> >> 
> >> LS
> >
> >This thread has stalled, let's try to restart it.
> >
> >What versions are normally still being worked on by developers? I
> >suspect it's master and sssd-1-12. What about pushing the patch to
> >sssd-1-12 as well?
> I'm fine with sssd-1-12.
> The python sssdconfig need to be updated.
> 
> LS

Michal, if you agree, please update the patches so that we can push them
in time for sssd-1.13.1


More information about the sssd-devel mailing list