On (27/07/15 15:58), Michal Židek wrote:
> On 07/24/2015 09:51 PM, Lukas Slebodnik wrote:
>> On (07/07/15 19:45), Michal Židek wrote:
>>> Hi!
>>>
>>> See the attached patch.
>>>
>>> Ticket:
>>>
https://fedorahosted.org/sssd/ticket/2688
>>>
>>> CI passed:
>>>
http://sssd-ci.duckdns.org/logs/job/18/71/summary.html
>>>
>>> Thanks,
>>> Michal
>>>
>>> --
>>> Senior Principal Intern
>>
>> >From c67f8df99df4f1aafa41d57a84c8d1c19fb5d610 Mon Sep 17 00:00:00 2001
>>> From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzidek(a)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
>>> ---
>>> src/confdb/confdb.h | 1 +
>>> src/confdb/confdb_setup.c | 48
++++++++++++++--------------
>>> src/config/SSSDConfig/sssd_upgrade_config.py | 3 +-
>>> 3 files changed, 27 insertions(+), 25 deletions(-)
>>>
>> I think you can also remove config_file_version from
>> sssd.conf in integration test. Or at least comment it out.
>> So it will be proven that your patch works.
>>
>> LS
>
> Ok, I added patch that removes the option from
> sssd.conf in integration tests.
>
> Michal
>
> --
> Senior Principal Intern
>From af960ad090a06017ce3880abaee9c07bbfbd8111 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzidek(a)redhat.com>
> Date: Tue, 7 Jul 2015 15:15:32 +0200
> Subject: [PATCH 1/2] 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
> ---
> src/confdb/confdb.h | 1 +
> src/confdb/confdb_setup.c | 48 ++++++++++++++--------------
> src/config/SSSDConfig/sssd_upgrade_config.py | 3 +-
> 3 files changed, 27 insertions(+), 25 deletions(-)
>
> diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h
> index 36df6ae..d2d7ebf 100644
> --- a/src/confdb/confdb.h
> +++ b/src/confdb/confdb.h
> @@ -38,6 +38,7 @@
> * @{
> */
>
> +#define CONFDB_DEFAULT_CFG_FILE_VER 2
> #define CONFDB_FILE "config.ldb"
> #define CONFDB_DEFAULT_CONFIG_FILE SSSD_CONF_DIR"/sssd.conf"
> #define SSSD_MIN_ID 1
> diff --git a/src/confdb/confdb_setup.c b/src/confdb/confdb_setup.c
> index 93a1a1b..360e3a8 100644
> --- a/src/confdb/confdb_setup.c
> +++ b/src/confdb/confdb_setup.c
> @@ -224,30 +224,30 @@ int confdb_init_db(const char *config_file, struct confdb_ctx
*cdb)
>
> ret = sss_ini_check_config_obj(init_data);
> if (ret != EOK) {
> - /* No known version. Assumed to be version 1 */
> - DEBUG(SSSDBG_FATAL_FAILURE,
> - "Config file is an old version. "
> - "Please run configuration upgrade script.\n");
> - ret = EINVAL;
> - goto done;
> - }
> -
> - version = sss_ini_get_int_config_value(init_data, 1, -1, &ret);
> - if (ret != EOK) {
> - DEBUG(SSSDBG_FATAL_FAILURE,
> - "Config file version could not be determined\n");
> - goto done;
> - } 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");
> + goto done;
> + } 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;
> + }
> }
>
> /* Set up a transaction to replace the configuration */
> diff --git a/src/config/SSSDConfig/sssd_upgrade_config.py
b/src/config/SSSDConfig/sssd_upgrade_config.py
> index 282d6c4..767d06d 100644
> --- a/src/config/SSSDConfig/sssd_upgrade_config.py
> +++ b/src/config/SSSDConfig/sssd_upgrade_config.py
> @@ -47,7 +47,8 @@ class SSSDConfigFile(SSSDChangeConf):
> def get_version(self):
> ver = self.get_option_index('sssd',
'config_file_version')[1]
> if not ver:
> - return 1
> + # config_file_version not found -> default to version 2
> + return 2
> try:
> return int(ver['value'])
> except ValueError:
> --
> 2.1.0
>
>From 52fb274e36c104a4b7d2df932d81a7ae07c008fb Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzidek(a)redhat.com>
> Date: Mon, 27 Jul 2015 14:22:08 +0200
> Subject: [PATCH 2/2] tests: Remove config_file_version from sssd.conf in CI
>
> Default config_file_version was changed to 2 so
> the explicitly setting the value is no longer necessary.
> This change also serves as a test to see that the
> new default works.
>
> Ticket:
>
https://fedorahosted.org/sssd/ticket/2688
> ---
I prefer if so small change in test is part of "feature patch"
So you do not forget to backport it.