merge ticket #294 (friendlier error for incompatible/old configuration database)

also bumps the configuration file version


git-svn-id: svn://bind10.isc.org/svn/bind10/trunk@2695 e5f2f494-b856-4b98-b285-d166d9295462

src/bin/cfgmgr/b10-cfgmgr.py.in

@@ -19,7 +19,7 @@
 
 import sys; sys.path.append ('@@PYTHONPATH@@')
 
-from isc.config.cfgmgr import ConfigManager
+from isc.config.cfgmgr import ConfigManager, ConfigManagerDataReadError
 from isc.cc import SessionError
 import signal
 import os
@@ -52,10 +52,15 @@ def main():
     except SessionError as se:
         print("[b10-cfgmgr] Error creating config manager, "
               "is the command channel daemon running?")
+        return 1
     except KeyboardInterrupt as kie:
         print("[b10-cfgmgr] Interrupted, exiting")
+    except ConfigManagerDataReadError as cmdre:
+        print("[b10-cfgmgr] " + str(cmdre))
+        return 2
     if cm:
-        cm.write_config()
+        return cm.write_config()
+    return 0
 
 if __name__ == "__main__":
-    main()
+    sys.exit(main())

src/lib/config/Makefile.am

@@ -15,6 +15,7 @@ endif
 EXTRA_DIST =  testdata/b10-config-bad1.db
 EXTRA_DIST += testdata/b10-config-bad2.db
 EXTRA_DIST += testdata/b10-config-bad3.db
+EXTRA_DIST += testdata/b10-config-bad4.db
 EXTRA_DIST += testdata/b10-config.db
 EXTRA_DIST += testdata/data22_1.data
 EXTRA_DIST += testdata/data22_2.data

src/lib/config/testdata/b10-config-bad1.db

@@ -1 +1 @@
-{'version': 0}
+{"version": 0}

src/lib/config/testdata/b10-config-bad4.db

@@ -0,0 +1 @@
+{'version': 2}

src/lib/config/testdata/b10-config.db

@@ -1 +1 @@
-{"version": 1, "TestModule": {"test": 125}}
+{"version": 2, "TestModule": {"test": 125}}

src/lib/python/isc/config/ccsession.py

@@ -37,7 +37,7 @@
 """
 
 from isc.cc import Session
-from isc.config.config_data import ConfigData, MultiConfigData
+from isc.config.config_data import ConfigData, MultiConfigData, BIND10_CONFIG_DATA_VERSION
 import isc
 
 class ModuleCCSessionError(Exception): pass
@@ -333,7 +333,7 @@ class UIModuleCCSession(MultiConfigData):
         """Requests the current configuration from the configuration
            manager through b10-cmdctl, and stores those as CURRENT"""
         config = self._conn.send_GET('/config_data')
-        if 'version' not in config or config['version'] != 1:
+        if 'version' not in config or config['version'] != BIND10_CONFIG_DATA_VERSION:
             raise ModuleCCSessionError("Bad config version")
         self._set_current_config(config)
 

src/lib/python/isc/config/cfgmgr.py

@@ -27,7 +27,7 @@ import copy
 import tempfile
 import json
 from isc.cc import data
-from isc.config import ccsession
+from isc.config import ccsession, config_data
 
 class ConfigManagerDataReadError(Exception):
     """This exception is thrown when there is an error while reading
@@ -43,15 +43,13 @@ class ConfigManagerData:
     """This class hold the actual configuration information, and
        reads it from and writes it to persistent storage"""
 
-    CONFIG_VERSION = 1
-
     def __init__(self, data_path, file_name = "b10-config.db"):
         """Initialize the data for the configuration manager, and
            set the version and path for the data store. Initializing
            this does not yet read the database, a call to
            read_from_file is needed for that."""
         self.data = {}
-        self.data['version'] = ConfigManagerData.CONFIG_VERSION
+        self.data['version'] = config_data.BIND10_CONFIG_DATA_VERSION
         self.data_path = data_path
         self.db_filename = data_path + os.sep + file_name
 
@@ -65,21 +63,36 @@ class ConfigManagerData:
            the second exception, the best way is probably to report the
            error and stop loading the system."""
         config = ConfigManagerData(data_path, file_name)
+        file = None
         try:
             file = open(config.db_filename, 'r')
             file_config = json.loads(file.read())
-            if 'version' in file_config and \
-                file_config['version'] == ConfigManagerData.CONFIG_VERSION:
-                config.data = file_config
+            # handle different versions here
+            # If possible, we automatically convert to the new
+            # scheme and update the configuration
+            # If not, we raise an exception
+            if 'version' in file_config:
+                if file_config['version'] == config_data.BIND10_CONFIG_DATA_VERSION:
+                    config.data = file_config
+                elif file_config['version'] == 1:
+                    # only format change, no other changes necessary
+                    file_config['version'] = 2
+                    print("[b10-cfgmgr] Updating configuration database version from 1 to 2")
+                    config.data = file_config
+                else:
+                    if config_data.BIND10_CONFIG_DATA_VERSION > file_config['version']:
+                        raise ConfigManagerDataReadError("Cannot load configuration file: version %d no longer supported" % file_config['version'])
+                    else:
+                        raise ConfigManagerDataReadError("Cannot load configuration file: version %d not yet supported" % file_config['version'])
             else:
-                # We can put in a migration path here for old data
-                raise ConfigManagerDataReadError("[b10-cfgmgr] Old version of data found")
-            file.close()
+                raise ConfigManagerDataReadError("No version information in configuration file " + config.db_filename)
         except IOError as ioe:
-            raise ConfigManagerDataEmpty("No config file found")
-        except:
-            raise ConfigManagerDataReadError("Config file unreadable")
-
+            raise ConfigManagerDataEmpty("No configuration file found")
+        except ValueError:
+            raise ConfigManagerDataReadError("Configuration file out of date or corrupt, please update or remove " + config.db_filename)
+        finally:
+            if file:
+                file.close();
         return config
         
     def write_to_file(self, output_file_name = None):
@@ -102,11 +115,11 @@ class ConfigManagerData:
                 os.rename(filename, self.db_filename)
         except IOError as ioe:
             # TODO: log this (level critical)
-            print("[b10-cfgmgr] Unable to write config file; configuration not stored: " + str(ioe))
+            print("[b10-cfgmgr] Unable to write configuration file; configuration not stored: " + str(ioe))
             # TODO: debug option to keep file?
         except OSError as ose:
             # TODO: log this (level critical)
-            print("[b10-cfgmgr] Unable to write config file; configuration not stored: " + str(ose))
+            print("[b10-cfgmgr] Unable to write configuration file; configuration not stored: " + str(ose))
         try:
             if filename and os.path.exists(filename):
                 os.remove(filename)
@@ -243,7 +256,7 @@ class ConfigManager:
             except data.DataNotFoundError as dnfe:
                 # no data is ok, that means we have nothing that
                 # deviates from default values
-                return ccsession.create_answer(0, { 'version': self.config.CONFIG_VERSION })
+                return ccsession.create_answer(0, { 'version': config_data.BIND10_CONFIG_DATA_VERSION })
         else:
             return ccsession.create_answer(1, "Bad module_name in get_config command")
 

src/lib/python/isc/config/config_data.py

@@ -25,6 +25,8 @@ import isc.config.module_spec
 
 class ConfigDataError(Exception): pass
 
+BIND10_CONFIG_DATA_VERSION = 2
+
 def check_type(spec_part, value):
     """Does nothing if the value is of the correct type given the
        specification part relevant for the value. Raises an

src/lib/python/isc/config/tests/ccsession_test.py

@@ -22,6 +22,7 @@
 import unittest
 import os
 from isc.config.ccsession import *
+from isc.config.config_data import BIND10_CONFIG_DATA_VERSION
 from unittest_fakesession import FakeModuleCCSession
 
 class TestHelperFunctions(unittest.TestCase):
@@ -442,20 +443,20 @@ class TestUIModuleCCSession(unittest.TestCase):
     def create_uccs2(self, fake_conn):
         module_spec = isc.config.module_spec_from_file(self.spec_file("spec2.spec"))
         fake_conn.set_get_answer('/module_spec', { module_spec.get_module_name(): module_spec.get_full_spec()})
-        fake_conn.set_get_answer('/config_data', { 'version': 1 })
+        fake_conn.set_get_answer('/config_data', { 'version': BIND10_CONFIG_DATA_VERSION })
         return UIModuleCCSession(fake_conn)
 
     def test_init(self):
         fake_conn = fakeUIConn()
         fake_conn.set_get_answer('/module_spec', {})
-        fake_conn.set_get_answer('/config_data', { 'version': 1 })
+        fake_conn.set_get_answer('/config_data', { 'version': BIND10_CONFIG_DATA_VERSION })
         uccs = UIModuleCCSession(fake_conn)
         self.assertEqual({}, uccs._specifications)
-        self.assertEqual({ 'version': 1}, uccs._current_config)
+        self.assertEqual({ 'version': BIND10_CONFIG_DATA_VERSION}, uccs._current_config)
 
         module_spec = isc.config.module_spec_from_file(self.spec_file("spec2.spec"))
         fake_conn.set_get_answer('/module_spec', { module_spec.get_module_name(): module_spec.get_full_spec()})
-        fake_conn.set_get_answer('/config_data', { 'version': 1 })
+        fake_conn.set_get_answer('/config_data', { 'version': BIND10_CONFIG_DATA_VERSION })
         uccs = UIModuleCCSession(fake_conn)
         self.assertEqual(module_spec._module_spec, uccs._specifications['Spec2']._module_spec)
 

src/lib/python/isc/config/tests/cfgmgr_test.py

@@ -22,6 +22,7 @@
 import unittest
 import os
 from isc.config.cfgmgr import *
+from isc.config import config_data
 from unittest_fakesession import FakeModuleCCSession
 
 class TestConfigManagerData(unittest.TestCase):
@@ -32,7 +33,7 @@ class TestConfigManagerData(unittest.TestCase):
 
     def test_init(self):
         self.assertEqual(self.config_manager_data.data['version'],
-                         ConfigManagerData.CONFIG_VERSION)
+                         config_data.BIND10_CONFIG_DATA_VERSION)
         self.assertEqual(self.config_manager_data.data_path,
                          self.data_path)
         self.assertEqual(self.config_manager_data.db_filename,
@@ -52,6 +53,9 @@ class TestConfigManagerData(unittest.TestCase):
         self.assertRaises(ConfigManagerDataReadError,
                           ConfigManagerData.read_from_file,
                           self.data_path, "b10-config-bad3.db")
+        self.assertRaises(ConfigManagerDataReadError,
+                          ConfigManagerData.read_from_file,
+                          self.data_path, "b10-config-bad4.db")
 
     def test_write_to_file(self):
         output_file_name = "b10-config-write-test";
@@ -161,13 +165,13 @@ class TestConfigManager(unittest.TestCase):
         self.assertEqual(commands_spec['Spec2'], module_spec.get_commands_spec())
 
     def test_read_config(self):
-        self.assertEqual(self.cm.config.data, {'version': 1})
+        self.assertEqual(self.cm.config.data, {'version': config_data.BIND10_CONFIG_DATA_VERSION})
         self.cm.read_config()
         # due to what get written, the value here is what the last set_config command in test_handle_msg does
-        self.assertEqual(self.cm.config.data, {'TestModule': {'test': 125}, 'version': 1})
+        self.assertEqual(self.cm.config.data, {'TestModule': {'test': 125}, 'version': config_data.BIND10_CONFIG_DATA_VERSION})
         self.cm.data_path = "/no_such_path"
         self.cm.read_config()
-        self.assertEqual(self.cm.config.data, {'version': 1})
+        self.assertEqual(self.cm.config.data, {'version': config_data.BIND10_CONFIG_DATA_VERSION})
 
     def test_write_config(self):
         # tested in ConfigManagerData tests
@@ -190,9 +194,9 @@ class TestConfigManager(unittest.TestCase):
                                 {'result': [1, 'Bad get_module_spec command, argument not a dict']})
         self._handle_msg_helper({ "command": [ "get_module_spec", { } ] },
                                 {'result': [1, 'Bad module_name in get_module_spec command']})
-        self._handle_msg_helper({ "command": [ "get_config" ] }, { 'result': [ 0, { 'version': 1} ]})
+        self._handle_msg_helper({ "command": [ "get_config" ] }, { 'result': [ 0, { 'version': config_data.BIND10_CONFIG_DATA_VERSION } ]})
         self._handle_msg_helper({ "command": [ "get_config", { "module_name": "nosuchmodule" } ] },
-                                {'result': [0, { 'version': 1 }]})
+                                {'result': [0, { 'version': config_data.BIND10_CONFIG_DATA_VERSION }]})
         self._handle_msg_helper({ "command": [ "get_config", 1 ] },
                                 {'result': [1, 'Bad get_config command, argument not a dict']})
         self._handle_msg_helper({ "command": [ "get_config", { } ] },