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?= <[email protected]>
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/__init__.py.in | 5 ---
src/config/SSSDConfig/sssd_upgrade_config.py | 3 +-
src/config/SSSDConfigTest.py | 11 ++-----
src/tests/intg/ldap_test.py | 2 --
src/tests/intg/test_memory_cache.py | 3 --
7 files changed, 29 insertions(+), 44 deletions(-)
diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h
index 9aa2648..427c309 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..694a7f0 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/__init__.py.in b/src/config/SSSDConfig/__init__.py.in
index f10b58c..cb9c368 100644
--- a/src/config/SSSDConfig/__init__.py.in
+++ b/src/config/SSSDConfig/__init__.py.in
@@ -738,11 +738,6 @@ class SSSDService(SSSDConfigObject):
# Set up default options for this service
self.options.update(self.schema.get_defaults(self.name))
- # For the [sssd] service, force the config file version
- if servicename == 'sssd':
- self.options['config_file_version'] = 2
- self.hidden_options.append('config_file_version')
-
def list_options_with_mandatory(self):
"""
List options for the service, including the mandatory flag.
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:
diff --git a/src/config/SSSDConfigTest.py b/src/config/SSSDConfigTest.py
index 4aefe67..5047bd7 100755
--- a/src/config/SSSDConfigTest.py
+++ b/src/config/SSSDConfigTest.py
@@ -403,9 +403,6 @@ class SSSDConfigTestSSSDService(unittest.TestCase):
def testGetOption(self):
service = SSSDConfig.SSSDService('sssd', self.schema)
- # Positive test - Single-valued
- self.assertEqual(service.get_option('config_file_version'), 2)
-
# Positive test - List of values
self.assertEqual(service.get_option('services'), ['nss', 'pam'])
@@ -417,9 +414,7 @@ class SSSDConfigTestSSSDService(unittest.TestCase):
#Positive test
options = service.get_all_options()
- control_list = [
- 'config_file_version',
- 'services']
+ control_list = ['services']
self.assertTrue(type(options) == dict,
"Options should be a dictionary")
@@ -1266,9 +1261,7 @@ class SSSDConfigTestSSSDConfig(unittest.TestCase):
for section in sssdconfig.sections():
self.assertTrue(section['name'] in control_list)
- control_list = [
- 'config_file_version',
- 'services']
+ control_list = ['services']
for option in control_list:
self.assertTrue(sssdconfig.has_option('sssd', option),
"Option [%s] missing from [sssd]" %
diff --git a/src/tests/intg/ldap_test.py b/src/tests/intg/ldap_test.py
index bfe4e65..d01ebc7 100644
--- a/src/tests/intg/ldap_test.py
+++ b/src/tests/intg/ldap_test.py
@@ -116,7 +116,6 @@ def sanity_rfc2307(request, ldap_conn):
conf = unindent("""\
[sssd]
debug_level = 0xffff
- config_file_version = 2
domains = LDAP
services = nss, pam
@@ -174,7 +173,6 @@ def sanity_rfc2307_bis(request, ldap_conn):
conf = unindent("""\
[sssd]
debug_level = 0xffff
- config_file_version = 2
domains = LDAP
services = nss, pam
diff --git a/src/tests/intg/test_memory_cache.py b/src/tests/intg/test_memory_cache.py
index a333147..1f4ccb0 100644
--- a/src/tests/intg/test_memory_cache.py
+++ b/src/tests/intg/test_memory_cache.py
@@ -136,7 +136,6 @@ def sanity_rfc2307(request, ldap_conn):
conf = unindent("""\
[sssd]
- config_file_version = 2
domains = LDAP
services = nss
@@ -162,7 +161,6 @@ def fqname_rfc2307(request, ldap_conn):
conf = unindent("""\
[sssd]
- config_file_version = 2
domains = LDAP
services = nss
@@ -189,7 +187,6 @@ def fqname_case_insensitive_rfc2307(request, ldap_conn):
conf = unindent("""\
[sssd]
- config_file_version = 2
domains = LDAP
services = nss
--
2.1.0
_______________________________________________
sssd-devel mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
