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

Michal Židek mzidek at redhat.com
Tue Sep 1 10:55:29 UTC 2015


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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-CONFDB-Assume-config-file-version-2-if-missing.patch
Type: text/x-patch
Size: 7845 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20150901/71591eaa/attachment-0001.bin>


More information about the sssd-devel mailing list