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

Lukas Slebodnik lslebodn at redhat.com
Thu Sep 3 07:58:27 UTC 2015


On (03/09/15 09:35), Jakub Hrozek wrote:
>On Wed, Sep 02, 2015 at 02:52:42PM +0200, Lukas Slebodnik wrote:
>> On (01/09/15 12:55), Michal Židek wrote:
>> >On 09/01/2015 11:11 AM, Jakub Hrozek wrote:
>> >>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
>> >
>> >New patch attached. I tested ipa-client-install and the
>> >config_file_version is no longer added.
>> >
>> >Michal
>> >
>> 
>> >From f61ca88d69b3a56645b7473928b0674ac4bceff8 Mon Sep 17 00:00:00 2001
>> >From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzidek at redhat.com>
>> >Date: Tue, 7 Jul 2015 15:15:32 +0200
>> >Subject: [PATCH] CONFDB: Assume config file version 2 if missing
>> >
>> >Default to config file version 2 if the version
>> >is not specified explicitly.
>> >
>> >Ticket:
>> >https://fedorahosted.org/sssd/ticket/2688
>> >---
>> * integration tests passed
>> * ipa-client generated config without config_file_version and sssd worked.
>> 
>> ACK
>> 
>> LS
>
>* master: 175613be0cfb0890174d12d941e634d833b63dd9
>
>Michal, can you rebase the patch for sssd-1-12 ?
The conflict might me caused by missing integration tests in 1.12

LS


More information about the sssd-devel mailing list