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

Michal Židek mzidek at redhat.com
Mon Aug 10 15:18:01 UTC 2015


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
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
little inconveniences.

>
> Please also remove config_file_version from test_memory_cache.py
>

We do not necessary need to have all tests without
the option to test if the default works. But I removed
it from test_memory_cache.py anyway.

Michal

> LS
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>

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


More information about the sssd-devel mailing list